[PATCH] D19549: Fold compares irrespective of whether allocation can be elided
    Sanjoy Das via llvm-commits 
    llvm-commits at lists.llvm.org
       
    Mon May  2 11:19:41 PDT 2016
    
    
  
sanjoy requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: lib/Analysis/CaptureTracking.cpp:312
@@ -311,1 +311,3 @@
             break;
+      // Comparison against value stored in global variable.
+      // Given the pointer does not escape, it's value cannot be guessed and
----------------
Is this comment block wrapped to 80 chars?
================
Comment at: lib/Analysis/CaptureTracking.cpp:313
@@ +312,3 @@
+      // Comparison against value stored in global variable.
+      // Given the pointer does not escape, it's value cannot be guessed and
+      // stored separately in a global variable. If the pointer escapes while
----------------
Nit: "its"
================
Comment at: lib/Analysis/CaptureTracking.cpp:314
@@ +313,3 @@
+      // Given the pointer does not escape, it's value cannot be guessed and
+      // stored separately in a global variable. If the pointer escapes while
+      // visiting other instructions, we return from this function with
----------------
Nit: I'd remove the "If the pointer escapes ... bit".  I think the first sentence is enough, and the "captured = true" bit can be misleading since there isn't a `bool capture` in this function.
================
Comment at: lib/Analysis/InstructionSimplify.cpp:2100
@@ +2099,3 @@
+  if (MI && !PointerMayBeCaptured(MI, true, true))
+    return ConstantInt::get(GetCompareTy(LHS), !CmpInst::isTrueWhenEqual(Pred));
+
----------------
You can use `CmpInst::isFalseWhenEqual` here, but did you intend to put this under the previous `Pred == CmpInst::ICMP_EQ || Pred == CmpInst::ICMP_NE` check?  Right now you're handling inequalities too, I think.
Inequalities are tricky, since it is difficult to justify folding `%newPtr ULE -1` to `false`, for instance.
Repository:
  rL LLVM
http://reviews.llvm.org/D19549
    
    
More information about the llvm-commits
mailing list