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

Oliver Stannard via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 28 02:38:19 PDT 2015


olista01 marked 2 inline comments as done.

================
Comment at: include/llvm/Analysis/TargetLibraryInfo.h:249
@@ +248,3 @@
+      return false;
+    switch (F) {
+    default: break;
----------------
vsk wrote:
> It looks like many of these entries are shared with the switch in hasOptimizedCodeGen(). Could you consider sharing some of this code?
I'm not sure about this, the overlap between the two sets of functions doesn't have any meaning beyond "is side effect free and has optimised codegen", so I don't see what we gain by factoring it out.

================
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();
----------------
vsk wrote:
> 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 :).
I'm also not keen on the "IgnoringTermination" name, but I think non-termination should count as a side effect (unless ignoring it is safe for the given transformation, for example the change in SimplfyCFG.cpp), so I'd rather leave "mayHaveSideEffects" as the one that does take termination into account.


Repository:
  rL LLVM

http://reviews.llvm.org/D13137





More information about the llvm-commits mailing list