[PATCH] [ValueTracking] Allow min/max detection to see through casts.

David Majnemer david.majnemer at gmail.com
Wed May 13 23:47:13 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/Analysis/ValueTracking.cpp:3279
@@ +3278,3 @@
+
+static Constant *getTruncatedConstant(Constant *C, Type *ToType) {
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
----------------
reames wrote:
> 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.  
Right, how is this different from `ConstantExpr::getTrunc` ?

================
Comment at: lib/Analysis/ValueTracking.cpp:3290-3292
@@ +3289,5 @@
+    return ConstantVector::get(Cs);
+  } else {
+    return nullptr;
+  }
+}
----------------
No need for the extra indentation here, just stick return nullptr at the end of the function.

================
Comment at: lib/Analysis/ValueTracking.cpp:3295
@@ +3294,3 @@
+
+static Constant *getSExtConstant(Constant *C, Type *ToType) {
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
----------------
Likewise, why can't this be `ConstantExpr::getSExt`

================
Comment at: lib/Analysis/ValueTracking.cpp:3311
@@ +3310,3 @@
+
+static Constant *getZExtConstant(Constant *C, Type *ToType) {
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
----------------
Likewise, why can't this be `ConstantExpr::getZExt`

================
Comment at: lib/Analysis/ValueTracking.cpp:3327
@@ +3326,3 @@
+
+SelectPatternFlavor llvm::matchSelectPattern(Value *V,
+                                             Value *&LHS, Value *&RHS,
----------------
It'd be nice if `V` was const qualified.

================
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);
----------------
reames wrote:
> This loop usage is a bit confusing.  It would probably be clearer to extract out a helper function and call it twice.  
Agreed.

http://reviews.llvm.org/D9748

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list