[PATCH] D32285: [InstCombineCasts] Fix checks in sext->lshr->trunc pattern.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 9 07:46:40 PDT 2017


spatel added a comment.

Ok, your explanation makes this clearer. Since we're fixing a miscompile, I think it's ok to keep this patch mostly as-is. But please add some FIXME notes and rebase this patch after https://reviews.llvm.org/rL302475:

1. This code is trying to do too much in one place IMO. It (and the related zext transform above it) should be split off into a helper function so that the (common?) case where "ASize" is the same as the final size (CI.getType()) are handled separately. When those types match, we do not need to check hasOneUse as strictly. We're also wastefully calling CastInst::CreateIntegerCast() in that case.
2. We've artificially excluded vector types by using m_ConstantInt().
3. As I showed in the Alive example, we can handle the case where (ShiftAmt > SExtSize - ASize) by adding an 'and' mask. This transform makes sense with appropriate one-use checking.
4. We can use m_OneUse() with the match() calls to make the code cleaner.


https://reviews.llvm.org/D32285





More information about the llvm-commits mailing list