[PATCH] D13137: Do not remove calls to functions that may not halt

[Vedant Kumar via llvm-commits]

vsk added a subscriber: vsk.
vsk added a comment.

Thanks, some minor comments inline --


================
Comment at: include/llvm/Analysis/TargetLibraryInfo.h:249
@@ +248,3 @@
+      return false;
+    switch (F) {
+    default: break;
----------------
It looks like many of these entries are shared with the switch in hasOptimizedCodeGen(). Could you consider sharing some of this code?

================
Comment at: include/llvm/IR/Instruction.h:394
@@ -388,2 +393,3 @@
   /// matters, isSafeToSpeculativelyExecute may be more appropriate.
   bool mayHaveSideEffects() const {
+    return mayWriteToMemory() || mayThrow() || mayNotReturn();
----------------
If you're open to an alternate name for this, how about "mayHaveSideEffectsOrNotTerminate"? Then "mayHaveSideEffectsIgnoringTermination" could be changed to "mayHaveSideEffects". IMO "IgnoringX" is a little hard to read. Up to you of course :).

================
Comment at: include/llvm/Transforms/Utils/Local.h:66
@@ -65,1 +65,3 @@
 
+/// isInstructionSideEffectFree - Return true if the instruction might have side
+/// effects.
----------------
"might have" -> "cannot have"?

================
Comment at: lib/Transforms/Utils/Local.cpp:360
@@ +359,3 @@
+  return isInstructionSideEffectFree(I, TLI);
+
+}
----------------
nit - extra line here


Repository:
  rL LLVM

http://reviews.llvm.org/D13137