erichkeane added a comment.
Cant help with the RT/LLVM parts, but the clang parts are pretty much right.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13278
+llvm::Value *
+CodeGenFunction::EmitX86CpuSupports(std::array<uint32_t, 4> FeatureMask) {
Value *Result = Builder.getTrue();
----------------
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.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13311
+ llvm::Constant *CpuFeatures2 =
+ CGM.CreateRuntimeVariable(ATy, "__cpu_features2");
+ cast<llvm::GlobalValue>(CpuFeatures2)->setDSOLocal(true);
----------------
This won't double-create this if used more than 1x, right? There doesn't need
to be something like GetOrCreate... here?
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13272
+ uint64_t Mask = llvm::X86::getCpuSupportsMask(FeatureStrs);
+ uint32_t FeaturesMask[4] = {uint32_t(Mask), uint32_t(Mask >> 32), 0, 0};
+ return EmitX86CpuSupports(FeaturesMask);
----------------
MaskRay wrote:
> pengfei wrote:
> > Use `Lo_32(Mask)`, `Hi_32(Mask)`?
> Lo_32/Hi_32 are currently unused in clang/. Using this needs MathExtras.h. I
> am unsure it's worthwhile..
I was previously using Lo_32/Hi_32 here (see the removed parts of
EmitX86CpuSupports). Were we using those transitively?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158329/new/
https://reviews.llvm.org/D158329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
