[llvm] [InstCombine] Make the `(icmp eq/ne (and X, Y), X)` canonicalization work for non-const operands (PR #84688)

via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 11 23:02:30 PDT 2024


================
@@ -7221,6 +7219,11 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
           foldICmpCommutative(I.getSwappedPredicate(), Op1, Op0, I))
     return Res;
 
+  // Need this to be after foldICmpCommutative so we do mask folds before
+  // transforming the `and`.
----------------
goldsteinn wrote:

@nikic 
Okay, Ive looked into this fair bit. I am back on the side this is more canonical. I have patches lined up to cleanup some of the regessions.
The remaining ones run into basically the following issue. `not` instructions as a tool for transforms are a fixed resource. If a transform expected to be able to use `not` instructions more profitably, I think it makes sense to order it first.

Some of these cases (non-single-use not) could theoretically be undone by iterating the use list of `V` in `getFreelyInverted`, but that seems a bit overkill.

Other than that, I don't really see a better approach that "try to order the most profitable first".

My preference would be to get some patches up that cover holes in the current folding that this patch highlighted, then submit this patch with the explicit "after `foldICmpWithLowBitMaskedVal`"

Is that reasonable?

https://github.com/llvm/llvm-project/pull/84688


More information about the llvm-commits mailing list