From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 42B6F385841D for ; Thu, 28 Jul 2022 09:58:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 42B6F385841D Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-456-MValvv5tOKeLs0IqELofiA-1; Thu, 28 Jul 2022 05:57:55 -0400 X-MC-Unique: MValvv5tOKeLs0IqELofiA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4C722811E76; Thu, 28 Jul 2022 09:57:55 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 03E9C2166B26; Thu, 28 Jul 2022 09:57:54 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 26S9vqMr2477244 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 28 Jul 2022 11:57:52 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 26S9vprx2477168; Thu, 28 Jul 2022 11:57:51 +0200 Date: Thu, 28 Jul 2022 11:57:50 +0200 From: Jakub Jelinek To: philip.herron@embecosm.com Cc: gcc-patches@gcc.gnu.org, herron.philip@googlemail.com, SimplyTheOther Subject: Re: [PATCH Rust front-end v1 2/4] Add Rust lang TargetHooks for i386 and x86_64 Message-ID: Reply-To: Jakub Jelinek References: <20220727134040.843750-1-philip.herron@embecosm.com> <20220727134040.843750-3-philip.herron@embecosm.com> MIME-Version: 1.0 In-Reply-To: <20220727134040.843750-3-philip.herron@embecosm.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jul 2022 09:58:03 -0000 Note, the ChangeLog entry formatting is wrong and in various places the code formatting as well. In gcc/rust/ you can choose different formatting if there are strong reasons for that, but at least it should be consistent and ideally documented. None of the gcc/doc, gcc/config, gcc/config/i386 directories have ChangeLog files, so all this should go into gcc/ChangeLog and filenames should be relative to the gcc/ directory, so doc/tm.texi.in, config/default-rust.cc, config/i386/crtdll.h etc. After : there should be a capital letter and the description what changed should end with a dot. Where possible, before : there should be space ( followed by what has changed followed by ). When some file is regenerated, just write Regenerated. or so. When a new file is added, just write New. or New file. On Wed, Jul 27, 2022 at 02:40:38PM +0100, herron.philip--- via Gcc-patches wrote: > gcc/doc/ChangeLog: > > * tm.texi.in: specifiy hooks for rust target info > * tm.texi: commit the generated documentation > > gcc/ChangeLog: > > * Makefile.in: add target to generate rust hooks > * config.gcc: add rust target interface to be compiled for i386 > * genhooks.cc: generate rust target hooks > * configure: autoconf update > * configure.ac: add tm_rust_file_list and tm_rust_include_list > > gcc/config/ChangeLog: > > * default-rust.cc: new target hooks initializer for rust > * gnu.h: add new macro GNU_USER_TARGET_RUST_OS_INFO > * dragonfly.h: define TARGET_RUST_OS_INFO > * freebsd-spec.h: define FBSD_TARGET_RUST_OS_INFO > * freebsd.h: define guard for TARGET_RUST_OS_INFO > * fuchsia.h: define TARGET_RUST_OS_INFO > * kfreebsd-gnu.h: define GNU_USER_TARGET_RUST_OS_INFO > * kopensolaris-gnu.h: define GNU_USER_TARGET_RUST_OS_INFO > * linux-android.h: define ANDROID_TARGET_RUST_OS_INFO > * linux.h: define GNU_USER_TARGET_RUST_OS_INFO > * netbsd.h: define NETBSD_TARGET_RUST_OS_INFO > * openbsd.h: define OPENBSD_TARGET_RUST_OS_INFO > * phoenix.h: define TARGET_RUST_OS_INFO > * sol2.h: define TARGET_RUST_OS_INFO > * vxworks.h: define VXWORKS_TARGET_RUST_OS_INFO > * vxworksae.h: define VXWORKS_TARGET_RUST_OS_INFO > > gcc/config/i386/ChangeLog: > > * crtdll.h: define EXTRA_TARGET_RUST_OS_INFO > * cygming.h: define TARGET_RUST_OS_INFO > * cygwin.h: define EXTRA_TARGET_RUST_OS_INFO > * darwin.h: define TARGET_RUST_OS_INFO > * djgpp.h: likewise > * gnu-user-common.h: define TARGET_RUST_OS_INFO > * i386-protos.h: prototype for ix86_rust_target_cpu_info > * i386-rust.cc: new file to generate the rust target host info > * i386.h: define TARGET_RUST_CPU_INFO hook > * linux-common.h: define hooks for target info > * lynx.h: likewise > * mingw32.h: likewise > * netbsd-elf.h: likewise > * netbsd64.h: likewise > * nto.h: likewise > * openbsdelf.h: likewise > * rdos.h: likewise > * rtemself.h: likewise > * t-i386: add makefilke rule for i386-rust.cc > * vxworks.h: define TARGET_RUST_OS_INFO > > gcc/rust/ChangeLog: > > * rust-target-def.h: define the headers to access the hooks > * rust-target.def: define the hooks nessecary based on C90 > * rust-target.h: define extern gcc_targetrustm > +/* Implement TARGET_RUST_CPU_INFO for x86 targets. */ > + > +void > +ix86_rust_target_cpu_info (void) > +{ > + if (TARGET_64BIT) { > + rust_add_target_info("target_arch", "x86_64"); The indentation should be just 2 columns rather than 4, and { doesn't go at the end of line. Single statement if/else/etc. bodies aren't wrapped with {}s. There is space before (. So if (TARGET_64BIT) { rust_add_target_info ("target_arch", "x86_64"); ... } else rust_add_target_info ("target_arch", "x86"); > + // nopl: hard-coded (as gcc doesn't technically have feature) to return true for cpu arches with it > + // maybe refactor into switch if multiple options > + bool hasNOPL = ix86_arch == PROCESSOR_PENTIUMPRO || ix86_arch == PROCESSOR_PENTIUM4 > + || ix86_arch == PROCESSOR_NOCONA || ix86_arch == PROCESSOR_CORE2 || ix86_arch == PROCESSOR_NEHALEM Some lines are too long. Jakub