[PATCH] D24414: Conditionally eliminate library calls where the result value is not used

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:131
@@ +130,3 @@
+  // Create a single condition.
+  Value *createCond(CallInst *CI, CmpInst::Predicate Cmp, float Val) {
+    IRBuilder<> BBBuilder(CI);
----------------
Make this one also takes IRBuilder<>& as parameter. Introduce another overload of CreateCond that does not have IRBuilder<>& 

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:149
@@ +148,3 @@
+  switch (Func) {
+  case LibFunc::asinh:
+  case LibFunc::asinhf:
----------------
why not having a separate patch to improve FE?

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:161
@@ +160,3 @@
+  case LibFunc::tanhl:
+  // The following functions seem already being marked by FE.
+  case LibFunc::ceil:
----------------
Why not using attribute marked by FE?

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:369
@@ +368,3 @@
+    return;
+  if (!CI.use_empty())
+    return;
----------------
If the runtime exposes API to invoke fast path no-errorno implementation, this pass can also be enhanced to handle calls that have uses. Perhaps add a comment here for future enhancement

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:471
@@ +470,3 @@
+// For pow(x,y), We only handle the following cases:
+// (1) x is a constant && (x >= 1) && (x < MaxUInt8)
+//     Cond is: (y > 127)
----------------
Add a comment that the condition can be more conservative than it actually is.

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:485
@@ +484,3 @@
+                                       const LibFunc::Func &Func) {
+  // LibFunc::powf and powl TBD.
+  if (Func != LibFunc::pow) {
----------------
// FIXME

================
Comment at: lib/Transforms/Utils/CondDeadCallElimination.cpp:595
@@ +594,3 @@
+    return false;
+  if (F.hasFnAttribute(Attribute::OptimizeForSize))
+    OptForSize = true;
----------------
Should OptimizeForSize be passed in via pass builder (see LoopUnswitch) as well?


https://reviews.llvm.org/D24414