[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