[Openmp-commits] [PATCH] D125717: [InstCombine] Optimize and of icmps with power-of-2 and contiguous masks
Noah Goldstein via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Sat May 20 17:20:24 PDT 2023
goldstein.w.n added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:610
/// Try to fold (icmp(A & B) ==/!= C) &/| (icmp(A & D) ==/!= E)
-/// into a single (icmp(A & X) ==/!= Y).
+/// into a single (icmp(A & X) ==/!= Y) or (icmp A u< Y).
static Value *foldLogOpOfMaskedICmps(ICmpInst *LHS, ICmpInst *RHS, bool IsAnd,
----------------
jmciver wrote:
> goldstein.w.n wrote:
> > When do we transform to `A & X eq/ne Y`? I don't see that code.
> That is a great question. The `A & X eq/ne Y` is emitted by the other transformations in `foldLogOpOfMaskedICmps`. When I started developing this patch I picked this location as the classifier function `getMaskedTypeForICmpPair` converted the source to the form:
> `(icmp ne (A & B), C) | (icmp qe (A & D), E)`. This is violation of the original intended use `foldLogOpOfMaskedICmps` at it is targetting ICMPs of the same operator, which fold to `(icmp(A & X) ==/!= Y)`.
>
> My current thinking is to move this fold into one of the other called functions in `foldAndOrOfICmps` (option 1) or create its own function and then call that from `foldAndOrOfICmps` (option 2).
>
> Option 1: One possible function is `foldIsPowerOf2`, but it is specific to `ctpop` based expressions so I am hesitant to expand functionality.
>
> Option 2: Create a function called `foldIsNegativePowerOf2` (open to other names), which will attempt the transform `X <u 2^n` to `(X & ~(2^n-1)) == 0` and then perform the subsequent tests for folding as seen in `foldLogOpOfMaskedICmpsAllZerosBMaskNeqMixedOrContiguous`. This removes the need for verbose aforementioned function name and provides a concise "does one thing well" function.
>
> I think option 2 is probably the best path. I'll go ahead and implement and see what it looks like. If you have other ideas let me know!
Bah, I had left reply unsent. Agree option 2 is better (and looks like thats what you did so great)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125717/new/
https://reviews.llvm.org/D125717
More information about the Openmp-commits
mailing list