[PATCH] Add NSW/NUW flags in InstCombine

[Joey Gouly]

ping

-----Original Message-----
From: Joey Gouly [mailto:joey.gouly at arm.com] 
Sent: 20 March 2014 17:16
To: 'Duncan P. N. Exon Smith';
reviews+D2426+public+060fa5c8b6d746c2 at llvm-reviews.chandlerc.com
Cc: llvm-commits at cs.uiuc.edu
Subject: RE: [PATCH] Add NSW/NUW flags in InstCombine

Sorry about the (really) late reply.

> I have a few points regarding the implementation details of the patch, but
first:  I'm not clear on the benefit.  AFAIK, the nuw/nsw flags just serve
to provide poison values when overflow leads to
> undefined behavior.  If we know these operations won't overflow, why add
the flags?

As far as I know, there is no other flags to denote more specifically that
overflow will not happen - but adding the nuw/nsw flags can help later
passes assume that no overflow will happen in this instruction, otherwise
the results can be undefined.  Adding these flags when creating a new
instruction seems like a good idea, besides WillNotOverflowSignedAdd already
exists in instcombine, so it felt natural to extend this functionality. I am
not sure what you mean by "we know these operations won't overflow". Are you
suggesting that every piece of code that relies on nsw/nuw flags should
duplicate this functionality for checking whether overflow is impossible
instead? (Comment from Georgia, who wrote the patch)


We addressed all the other comments you made!

Updated patch is attached.

Does it look good now? Anyone else got any comments?

Joey