[llvm] [GISel][CombinerHelper] Combine and(trunc(x), trunc(y)) -> trunc(and(x, y)) (PR #89023)

Dhruv Chawla via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 17 06:50:36 PDT 2024


dc03-work wrote:

> Why is this a good idea? Intuitively it means we'll do the `and` in a wider type than necessary, which could end up generating more code if the wider type is wider than a register. Is it a canonicalization? Does it enable other optimizations? Is there a general strategy to pull truncs out of other expressions?

1. This fold is something I noticed while looking into the s1279 kernel in the TSVC benchmarks for AArch64. GlobalISel was generating two truncs (actually one trunc and the other with much worse codegen, which I am trying to address with #88469), whereas SelectionDAG was generating only one.
2. I agree that this can be an issue for wider types. I was only looking from the perspective of legal vector types, sorry.
3. I don't know if it is a canonicalization, though I feel it should be when x and y have legal types that can be represented with one register, because of the following point:
4. This does enable a separate optimization when `ext(and(trunc, trunc))` occurs, where `ext(trunc(and))` can generally be folded to a simpler form. This did occur in the s1279 kernel, and is one of the reasons I made this patch.
5. Not sure.

As in my comment above, I am planning on moving this code into `matchHoistLogicOpWithSameOpcodeHands`. Maybe some special handling for `trunc` can also be added to check for point 3.

> > Hmm, one of the issues is that we need to match the types as well (the types of x and y in trunc(x) and trunc(y) need to be the same) so that the and(x, y) is well formed. I am not sure if this is possible through TableGen.
> 
> If we need to add something to support this, we should add the support to tablegen. This is not an uncommon type of scenario

I think that is beyond the scope of this patch, and is probably not possible to do immediately. We also need to check if the node we are generating is legal if we are not in the pre-legalizer combiner, as well as checking if the nodes have one non-dbg use. Not sure if TableGen should be extended to handle these cases as well.

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


More information about the llvm-commits mailing list