From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7833 invoked by alias); 23 Apr 2012 20:41:40 -0000 Received: (qmail 7823 invoked by uid 22791); 23 Apr 2012 20:41:38 -0000 X-SWARE-Spam-Status: No, hits=-4.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_ZJ X-Spam-Check-By: sourceware.org Received: from mail-qa0-f43.google.com (HELO mail-qa0-f43.google.com) (209.85.216.43) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 23 Apr 2012 20:41:25 +0000 Received: by qadb15 with SMTP id b15so2373749qad.9 for ; Mon, 23 Apr 2012 13:41:24 -0700 (PDT) MIME-Version: 1.0 Received: by 10.224.210.66 with SMTP id gj2mr776060qab.3.1335213683928; Mon, 23 Apr 2012 13:41:23 -0700 (PDT) Received: by 10.229.136.69 with HTTP; Mon, 23 Apr 2012 13:41:23 -0700 (PDT) In-Reply-To: References: <20120330001021.9E30EB2086@azwildcat.mtv.corp.google.com> Date: Mon, 23 Apr 2012 20:41:00 -0000 Message-ID: Subject: Re: Support for Runtime CPU type detection via builtins (issue5754058) From: "H.J. Lu" To: Sriraman Tallam Cc: Uros Bizjak , Richard Guenther , Michael Matz , reply@codereview.appspotmail.com, gcc-patches@gcc.gnu.org, Richard Henderson , Jan Hubicka Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-04/txt/msg01414.txt.bz2 On Mon, Apr 23, 2012 at 9:59 AM, Sriraman Tallam wrot= e: > Hi, > > On Mon, Apr 23, 2012 at 1:19 AM, Uros Bizjak wrote: >> On Tue, Apr 3, 2012 at 9:47 PM, Sriraman Tallam wr= ote: >> >>> i386 maintainers - =A0Is this patch ok? >> >> Has the community reached the consensus on how this kind of >> functionality has to be implemented? I have followed the discussion a >> bit, but IIRC, there was no clear decision. Without this decision, I >> am not able to review the _implementation_ of agreed functionality for >> x86 target. >> >> (I apologize if I have missed the decision, please point me to the >> discussion in this case.) > > The discussions are here: > > http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01446.html > and follow-ups to this. > > I am not sure about consensus, but the important points raised were: > > 1) Constructor ordering: What if some constructors fire before > cpu_indicator_init?, which determines the CPU. I addressed this > problem by making the priority of cpu_indicator_init to be the highest > possible. Still, IFUNC initializers will fire before and they have to > explicitly call __builtin_cpu_init() before checking the CPU type. > 2) Reducing the number of builtins : It is only two now. > > >> >>>>>> Patch available for review here: >>>>>> http://codereview.appspot.com/5754058 >> >> Please attach patches or inline it in the message itself for a review. >> Please see [1] for further instructions. > > Patch attached, tested on x86_64 and all tests pass. > > > =A0 =A0 =A0 * config/i386/i386.c (build_processor_features_struct): New f= unction. > =A0 =A0 =A0 (build_processor_model_struct): New function. > =A0 =A0 =A0 (make_var_decl): New function. > =A0 =A0 =A0 (get_field_from_struct): New function. > =A0 =A0 =A0 (fold_builtin_target): New function. > =A0 =A0 =A0 (ix86_fold_builtin): New function. > =A0 =A0 =A0 (ix86_expand_builtin): Expand new builtins by folding them. > =A0 =A0 =A0 (make_cpu_type_builtin): New functions. > =A0 =A0 =A0 (ix86_init_platform_type_builtins): Make the new builtins. > =A0 =A0 =A0 (ix86_init_builtins): Make new builtins to detect CPU type. > =A0 =A0 =A0 (TARGET_FOLD_BUILTIN): New macro. > =A0 =A0 =A0 (IX86_BUILTIN_CPU_INIT): New enum value. > =A0 =A0 =A0 (IX86_BUILTIN_CPU_IS): New enum value. > =A0 =A0 =A0 (IX86_BUILTIN_CPU_SUPPORTS): New enum value. > =A0 =A0 =A0 * config/i386/i386-builtin-types.def: New function type. > =A0 =A0 =A0 * testsuite/gcc.target/builtin_target.c: New testcase. > =A0 =A0 =A0 * doc/extend.texi: Document builtins. > > =A0 =A0 =A0 * libgcc/config/i386/i386-cpuinfo.c: New file. > =A0 =A0 =A0 * libgcc/config/i386/t-cpuinfo: New file. > =A0 =A0 =A0 * libgcc/config.host: Include t-cpuinfo. > =A0 =A0 =A0 * libgcc/config/i386/libgcc-glibc.ver: Version symbols __cpu_= model > =A0 =A0 =A0 and __cpu_features. > > Thanks, > -Sri. > > I have 2 comments: 1. You should remove static int called =3D 0; if (called) return 0; else called =3D 1; Instead, you can just do if (_cpu_model.__cpu_vendor) return 0; 2. You can replace if (vendor =3D=3D SIG_AMD) with else if (vendor =3D=3D SIG_AMD) Thanks. --=20 H.J.