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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13278
+llvm::Value *
+CodeGenFunction::EmitX86CpuSupports(std::array<uint32_t, 4> FeatureMask) {
   Value *Result = Builder.getTrue();
----------------
erichkeane wrote:
> Hmm... I guess size-wise this is on the edge of "const ref vs pass by value".  I think its fine now, but 'next time' this grows we'll have to think about making this a const-ref.
Yes. Right now passing 2 registers on a 64-bit target is more efficient.
'next time' may be quite a while from now:)


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13311
+  llvm::Constant *CpuFeatures2 =
+      CGM.CreateRuntimeVariable(ATy, "__cpu_features2");
+  cast<llvm::GlobalValue>(CpuFeatures2)->setDSOLocal(true);
----------------
erichkeane wrote:
> This won't double-create this if used more than 1x, right?  There doesn't need to be something like GetOrCreate... here?
`CodeGenModule::CreateRuntimeVariable` calls GetOrCreate internally. It's fine to be called multiple times.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158329