[PATCH] D55735: [InstSimplify] Simplify saturating add/sub + icmp
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 17 08:40:40 PST 2018
nikic marked an inline comment as done.
nikic added inline comments.
================
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;
----------------
spatel wrote:
> 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?
If the semantics aren't clear, it would be good to clarify in LangRef. However, right now I'm not sure I understand where the ambiguity lies. What result would you expect for this operation?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55735/new/
https://reviews.llvm.org/D55735
More information about the llvm-commits
mailing list