[PATCH] D139163: Utils: Add utility pass to lower ifuncs

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LowerIFunc.cpp:21
+/// Replace all call users of ifuncs in the module.
+static bool lowerIFuncs(Module &M) {
+  // SmallVector<GlobalIFunc *, 32> IFuncs(M.ifunc_begin(), M.ifunc_end());
----------------
this extra function seems unnecessary


================
Comment at: llvm/lib/Transforms/Utils/LowerIFunc.cpp:22
+static bool lowerIFuncs(Module &M) {
+  // SmallVector<GlobalIFunc *, 32> IFuncs(M.ifunc_begin(), M.ifunc_end());
+  if (M.ifunc_empty())
----------------
leftover code?


================
Comment at: llvm/lib/Transforms/Utils/ModuleUtils.cpp:385
+  // constructors?
+  const int Priority = -1;
+  appendToGlobalCtors(M, NewCtor, Priority,
----------------
making this positive is probably less likely to go wrong if some backend assumes non-negative priorities. I think 10 is probably good?


================
Comment at: llvm/test/Transforms/LowerIFunc/ifunc-constantexpr.ll:3
+; RUN: not opt -S -passes=lower-ifunc < %s 2>&1 | FileCheck %s
+; CHECK: IFunc must have a Function resolver
+
----------------
this is a verifier test? and should already be in tree as one


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

https://reviews.llvm.org/D139163