chandlerc added inline comments.
================
Comment at: lib/Basic/Targets/X86.cpp:844-845
- // FIXME: Historically, we defined this legacy name, it would be nice to
- // remove it at some point. We've never exposed fine-grained names for
- // recent primary x86 CPUs, and we should keep it that way.
- defineCPUMacros(Builder, "corei7");
----------------
craig.topper wrote:
> chandlerc wrote:
> > This seems to undo the idea that we should keep avoiding exposing
> > fine-grained CPU names? What's new that changes this?
> CPUs newer than the ones with that comment seem to have ignored said comment.
>
> Probably be cause we don't have a definition for what to do for new CPUs if
> we aren't going to expose fine grained names. Do we just call everything
> corei7 forever?
My hope would have been that people use *ISA*-based macros rather than anything
w/ a specific CPU. So I would have *removed* the corei7 for future CPUs and
simply not defined anything for them at all.
I think exposing something beyond ISA featureset through preprocessor macros is
questionable at best. I would at least want to see concrete use cases first.
================
Comment at: lib/Basic/Targets/X86.cpp:852
+ defineCPUMacros(Builder, "core_avx2");
+ defineCPUMacros(Builder, "haswell");
break;
----------------
craig.topper wrote:
> chandlerc wrote:
> > I find calling a Westmere CPU `nehalem` a little odd. Calling IvyBridge a
> > `sandybridge' CPU seems quite confusing. But calling Skylake (client) and
> > Cannonlake (all? client?) `haswell` seems .... deeply weird.
> This implementation matches what gcc does. I agree its weird.
>
> gcc doesn't implement cannonlake yet so i don't know what they'll do.
Ok, but maybe we should ask GCC to stop doing this. ;] We could talk to them
and try to figure out the right strategy is. My proposed strategy (see other
comment) is to restrict macros to ISA facilities.
https://reviews.llvm.org/D38824
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
