[PATCH] D25200: [InstCombine] New opportunities for FoldAndOfICmp and FoldXorOfICmp

Ehsan Amiri via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 5 11:58:37 PDT 2016


amehsan added a comment.

In https://reviews.llvm.org/D25200#562499, @spatel wrote:

> I know we want to expand the patterns that are matched beyond what's included in this patch, but it's not clear to me why we need IsAnyBitSet() for these cases. Eg, there are no tests for the 'icmp ugt' case.
>
> Is it simpler to just use more direct pattern matching for now and then see how it can be recoded as we add cases?
>
> I'm also wondering if there's some way to reuse the existing decomposeBitTestICmp() or the code in foldSelectInstWithICmp() that is marked with a FIXME. We also have isSignBitCheck() and isSignTest() in InstCombineCompares.cpp. There must be some way to refactor all of this logic because we're looking for similar patterns in several different places?


IsAnyBitSet factors out common pattern matching logic in FoldAndOFICmp and FoldXorOfICmp. I can add testcase for UGT case. DecomposeBitTestICmp will be helpful to be called for extending the current functionality. Currently IsAnyBitSet expects patterns similar to those generated by decomposeBitTestICmp so I believe we can first call decomposeBitTestICmp and then IsAnyBitSet to extend the functionality. I prefer to leave that as an extension of the current functionality.  isSignBitCheck is a similar but different functionality because it looks at a single special bit. IsAnyBitSet looks at masks that might have more than one bit. I also looked at the FIXME in simplifySelectWithICmpCond if that is what you mean. It says the logic is similar to decomposeBitTestICmp. So given what I said about decomposeBitTestICmp this is a separate issue.


https://reviews.llvm.org/D25200





More information about the llvm-commits mailing list