[PATCH] D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends

Tue May 14 09:35:32 PDT 2019

steven_wu added a comment.

Thanks for doing this. I think module flag is a good idea. Some comments inline.

================
Comment at: clang/test/CodeGen/svml-calls.ll:16
+
+define void @sin_f64(double* nocapture %varray) {
+; CHECK-LABEL: @sin_f64(
----------------
Personally, I think codegen tests like this will be cleaner to keep in LLVM. Clang tests just test the IRGen of the module flag and LLVM tests check that those flags are respected and module flag merge is respected.

================
Comment at: llvm/lib/LTO/LTOBackend.cpp:221

+static TargetLibraryInfoImpl *createTLII(Module &Mod, TargetMachine *TM) {
+  TargetLibraryInfoImpl *TLII =
----------------
Should this be done not just for LTOBackend but for regular compilation as well? LegacyCodegenerator and llc can all be benefit from a matching TargetLibraryInfo?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60162