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

David Majnemer david.majnemer at gmail.com
Thu May 14 10:48:11 PDT 2015


In http://reviews.llvm.org/D9748#172701, @jmolloy wrote:

> Hi Philip, David,
>
> Thanks for your review! I've updated the diff. I looked for public versions of those helper functions but couldn't find them - I didn't think to check ConstantExpr! :(
>
> All comments addressed. The only thing I'm not too keen on is the const_cast, I feel like raptors will come and attack me whenever it's used. But without it the "const" goes viral and takes over everything, including the return values "LHS" and "RHS".
>
> James


In that case, let's just skip the `const`.


REPOSITORY
  rL LLVM

================
Comment at: lib/Analysis/ValueTracking.cpp:3291-3293
@@ +3290,5 @@
+  case Instruction::SExt: {
+    Constant *Trunc = ConstantExpr::getTrunc(Op2, Castee->getType());
+    if (Trunc && ICI->isSigned())
+      return Trunc;
+    break;
----------------
Would it be equally correct to phrase this like:
  if (ICI->isSigned())
    if (Constant *Trunc = ConstantExpr::getTrunc(Op2, Castee->getType());
      return Trunc;

?

================
Comment at: lib/Analysis/ValueTracking.cpp:3297-3299
@@ +3296,5 @@
+  case Instruction::ZExt: {
+    Constant *Trunc = ConstantExpr::getTrunc(Op2, Castee->getType());
+    if (Trunc && !ICI->isSigned())
+      return Trunc;
+    break;
----------------
`!ICI->isSigned()` will return true for NE/EQ.  Perhaps:
  if (ICI->isUnsigned())
    if (Constant *Trunc = ConstantExpr::getTrunc(Op2, Castee->getType());
      return Trunc;

================
Comment at: lib/Analysis/ValueTracking.cpp:3302-3304
@@ +3301,5 @@
+  }
+  case Instruction::Trunc:
+    return ConstantExpr::getIntegerCast(Op2, Castee->getType(),
+                                        ICI->isSigned());
+  }
----------------
You may want to wrap the call to `getIntegerCast` with `if (!ICI->isEquality())` so you won't create the cast if the comparison is == or !=.

================
Comment at: lib/Analysis/ValueTracking.cpp:3328-3332
@@ +3327,7 @@
+      return ::matchSelectPattern(Pred, CmpLHS, CmpRHS,
+                                  cast<Instruction>(TrueVal)->getOperand(0), C,
+                                  LHS, RHS);
+    else if (Constant *C = lookThroughCast(ICI, FalseVal, TrueVal, CastOp))
+      return ::matchSelectPattern(Pred, CmpLHS, CmpRHS,
+                                  C, cast<Instruction>(FalseVal)->getOperand(0),
+                                  LHS, RHS);
----------------
I think it'd be more clear if you switched those `cast<Instruction>` over to `cast<CastInst>`.

http://reviews.llvm.org/D9748

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






More information about the llvm-commits mailing list