[PATCH] D55735: [InstSimplify] Simplify saturating add/sub + icmp

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 17 07:40:54 PST 2018


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/Analysis/InstructionSimplify.cpp:2681-2682
+    } else if (match(II.getOperand(1), m_APInt(C))) {
+      if (C->isNegative()) {
+        // ssub.sat(x, -C) produces [SINT_MIN - (-C), SINT_MAX]:
+        Lower = APInt::getSignedMinValue(Width) - *C;
----------------
nikic wrote:
> spatel wrote:
> > What happens if C is the signed min value here?
> > 
> > ```
> > define i1 @ssub_icmp_op1_is_min_val(i8 %a) {
> >   %b = call i8 @llvm.ssub.sat.i8(i8 %a, i8 -128)
> >   %c = icmp sle i8 %b, -118
> >   ret i1 %c
> > }
> > 
> > ```
> This will fold to `ret i1 false`. The computed range here is `[0, SINT_MAX]`. The last test is intended to check this case.
Ah, sorry I missed that test.

It's not obvious to me that the result of:
  %b = call i8 @llvm.ssub.sat.i8(i8 0, i8 -128)
is "127".

Is that worth noting here or in the LangRef?


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

https://reviews.llvm.org/D55735





More information about the llvm-commits mailing list