[PATCH] [ValueTracking] Allow min/max detection to see through	casts.
    Philip Reames 
    listmail at philipreames.com
       
    Wed May 13 09:40:44 PDT 2015
    
    
  
No obvious problems spotted, but I'd like to see the updated diff after comments applied so that I can be more confident of the change before signing off.
REPOSITORY
  rL LLVM
================
Comment at: lib/Analysis/ValueTracking.cpp:3211
@@ -3222,1 +3210,3 @@
+                                              Value *TrueVal, Value *FalseVal,
+                                              Value *&LHS, Value *&RHS) {
   LHS = CmpLHS;
----------------
Missing context here makes it hard to review change.  Please upload full diff.  
================
Comment at: lib/Analysis/ValueTracking.cpp:3279
@@ +3278,3 @@
+
+static Constant *getTruncatedConstant(Constant *C, Type *ToType) {
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
----------------
I find it odd that these routines don't already exist.  Have you looked at various subclasses of Constant and ConstantExpr?
If they don't already exist, I'd prefer to see them placed as members on Constant or an appropriate class.  
================
Comment at: lib/Analysis/ValueTracking.cpp:3344
@@ +3343,3 @@
+  if (CastOp && CmpLHS->getType() != TrueVal->getType()) {
+    for (int Swap = 0; Swap <= 1; ++Swap) {
+      CastInst *Op1 = dyn_cast<CastInst>(Swap ? FalseVal : TrueVal);
----------------
This loop usage is a bit confusing.  It would probably be clearer to extract out a helper function and call it twice.
http://reviews.llvm.org/D9748
EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
    
    
More information about the llvm-commits
mailing list