[PATCH] D59668: [ValueTracking] Take signedness into account for and/or ranges

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added inline comments.


================
Comment at: llvm/test/Transforms/InstSimplify/icmp-constant.ll:495-503
 ; -75 = 0b10110101, 53 = 0b00110101
 define i1 @and3_true1(i8 %x) {
 ; CHECK-LABEL: @and3_true1(
-; CHECK-NEXT:    [[Y:%.*]] = and i8 [[X:%.*]], -75
-; CHECK-NEXT:    [[Z:%.*]] = icmp sge i8 [[Y]], -75
-; CHECK-NEXT:    ret i1 [[Z]]
+; CHECK-NEXT:    ret i1 true
 ;
   %y = and i8 %x, -75
   %z = icmp sge i8 %y, -75
----------------
lebedev.ri wrote:
> This is a miscompile, i believe.
> https://rise4fun.com/Alive/R6k
> ```
> ----------------------------------------
> Optimization: 1
> 
> ERROR: Mismatch in values of i1 %z
> 
> Example:
> %x i8 = 0x80 (128, -128)
> %y i8 = 0x80 (128, -128)
> Source value: 0x0 (0)
> Target value: 0x1 (1, -1)
> ```
Wow, thanks for catching that! What I did with the lower bound for `and` here doesn't make sense, it should just be SignedMin, independent of the constant.

I'm wondering if I should maybe move these calculations to ConstantRange, so they can be independently (and exhaustively) tested. The calculations here are quite specific though, in that they compute the range when combining a full range and a single constant. Something like `ConstantRange::forBinaryAndWithConst()` and friends?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59668