[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

[Freddy, Ye via Phabricator via llvm-commits]

FreddyYe added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13319
+    Value *Features = Builder.CreateAlignedLoad(
+        Int32Ty, Builder.CreateGEP(ATy, CpuFeatures2, Idxs),
+        CharUnits::fromQuantity(4));
----------------
MaskRay wrote:
> FreddyYe wrote:
> > Will function multi version also be fixed in this patch? https://gcc.godbolt.org/z/cafhs9qbG If so, need to add test in clang/test/CodeGen/attr-target-mv.c
> The `target` attribute has strange semantics for overloading:
> ```
> int __attribute__((target("arch=skylake"))) foo(void) {return 0;}
> int __attribute__((target("arch=x86-64"))) foo(void) {return 1;}
> ```
> is not allowed in C mode of GCC.
> 
> I think such use cases are not recommended in C++.
> 
> If we don't use overloading, `int __attribute__((target("arch=x86-64"))) foo(void) {return 1;}` is supported by Clang today and this patch does not intend to change anything about it.
I think behavior for `target` attribute is not only overloading but also function multiversioning for redefined functions. And seems like C model of gcc haven't supported is due to it will target C23 standard: https://clang.llvm.org/docs/AttributeReference.html#target 
Comparing to gcc, clang misses not only `target` attribute but also other cpuname/feature related features for "x86-64*". See https://gcc.godbolt.org/z/arhne35GG (Seems gcc defined x86-64* as not only cpu name but also feature name.) Anyway, this patch is targeting for `target_clones` attribute only. So no problem here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158329/new/

https://reviews.llvm.org/D158329