[PATCH] D39483: [CVP] Remove some {s|u}{add|sub}.with.overflow checks.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

In https://reviews.llvm.org/D39483#919529, @jgalenson wrote:

> I've done the rename, although I'll note that I slightly prefer the old name since it makes it obvious that the function only applies to overflow intrinsics, not any intrinsic.


If you think the previous name was better I'm ok with that.

> As for changing ConstantRange, as I said before, I agree that should be done, but it seems to me like it should be done in a separate commit.  Once we have that, we should add a processSub (like processAdd), and we should add multiplication and division in all the places, but keeping focused commits seems like a better strategy to me.

I'm not suggesting you change ConstantRange in this commit (that indeed is not a great idea).  I'm suggesting that instead of adding a bunch of logic here only to move it to ConstantRange in a later commit, how about:

- Commit0: Only process uadd and sadd in CVP with ConstantRange::makeGuaranteedNoWrapRegion.
- Commit1: Teach makeGuaranteedNoWrapRegion about usub and ssub
- Commit2: Use Commit1 to process ssub and usub in CVP
- ...

If anything, the sequence above will make the commits even more focussed.  I hope I made sense.


https://reviews.llvm.org/D39483