[PATCH] D59506: [ValueTracking][InstSimplify] Support min/max selects in computeConstantRange()

Mon Mar 18 14:02:16 PDT 2019

nikic marked 2 inline comments as done.
nikic added inline comments.

================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5689-5692
+      Upper = *C + 1;
+      break;
+    case SPF_UMAX:
+      Lower = *C;
----------------
lebedev.ri wrote:
> Might be best not to hope that the other APInt is set to zero already?
This is the general convention in this code (the other two setLimitsFor functions): For unsigned ranges only one side is specified, because Lower/Upper are pre-initialized to a full unsigned range. For signed both have to be specified.

================
Comment at: llvm/test/Transforms/InstSimplify/cmp_of_min_max.ll:4

 define i1 @test_umin1(i32 %n) {
 ; CHECK-LABEL: @test_umin1(
----------------
spatel wrote:
> The min and max names for these tests seem inverted (ugt -> umax)?
You're right, min/max should be swapped in these tests.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59506/new/

https://reviews.llvm.org/D59506