tejohnson added a comment.
Great, thanks! A few minor comments below. Looks like it needs a clang-format
too.
================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:272
+ else
+ VectLibrary = Impl.ClVectorLibrary;
+
----------------
Suggest moving the implementation of this constructor to the .cpp file, in
which case you can just set VectLibrary directly from ClVectorLibrary there and
remove the member on the Impl object.
================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:318
bool AllowCallerSuperset) const {
if (!AllowCallerSuperset)
+ return VectLibrary == CalleeTLI.VectLibrary &&
----------------
This is set via a flag called "inline-caller-superset-nobuiltin". Suggest
changing the name to something like "inline-caller-superset-tli" to reflect new
larger scope. Also add a check with that option to your new inline test case.
================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:577
memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
- VectorDescs = TLI.VectorDescs;
- ScalarDescs = TLI.ScalarDescs;
+ for (unsigned i = 0; i < sizeof(TLI.VecLibDescs) /
sizeof(TLI.VecLibDescs[0]);
+ i++)
----------------
Why not just have "i < NumVecLibs"?
================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:590
AvailableArray);
- VectorDescs = TLI.VectorDescs;
- ScalarDescs = TLI.ScalarDescs;
+ for (unsigned i = 0; i < sizeof(TLI.VecLibDescs) /
sizeof(TLI.VecLibDescs[0]);
+ i++)
----------------
ditto
================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:1574
+ case NumVecLibs:
break;
}
----------------
Should these two be llvm_unreachable?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77632/new/
https://reviews.llvm.org/D77632
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
