[Codegen] Add the bitcast -> extui to shuffle folding patterns to EmulateNarrowTypes pass.

[MaheshRavishankar]

Folding the bitcast -> arith.extui to shuffle seems like worth doing across all backends (all backends support shuffle better). Also add a pattern to push broadcasts past extui-like operations to increase the coverage of cases where this kicks in.

[MaheshRavishankar]

Depends on llvm/llvm-project#68257

Towards #15091

[qedawkins]

This pattern was added to SPIR-V in #15029. If it works might be worth removing the now duplicate call there.

[dcaballe]

Would you mind elaborating on what is the code before and after the patch? A bitcast is a no-op and an integer extension should be faster than a shuffle. Do you have the generated asm so that we can understand what is going on?

[MaheshRavishankar]

Would you mind elaborating on what is the code before and after the patch? A bitcast is a no-op and an integer extension should be faster than a shuffle. Do you have the generated asm so that we can understand what is going on?

No vector integer extension operations go through a bad lowering path in LLVM. THere was an issue filed on it I think which led to the shuffle instruction path being built upstream by Nicolas. EDIT: It might be that the sub-byte integer extensions dont work well.

[dcaballe]

Do you have a small end-to-end repro that we can use to report this to LLVM? Not sure we want to keep this low level peephole patterns here in the long run. This is pretty low level.

[MaheshRavishankar]

See #14914

[qedawkins]

LGTM. I would wait for confirmation from @dcaballe first as well though.

[qedawkins]

darn, thanks for trying.

[MaheshRavishankar]

Do you have a small end-to-end repro that we can use to report this to LLVM? Not sure we want to keep this low level peephole patterns here in the long run. This is pretty low level.

Well maybe, but I don't see a harm. Shuffles are basic instructions and probably fairly well supported on all llvm backends/hardware. Reason I moved it here is because even on SPIRV doing shuffles this way works well.
This pass is run on all backends and is done during lowertollvm/NVvm/spirv. I suspect direct lowering of such kinds will grow with time since performance is more important now and we can't always avoid falling into bad spots in llvm.

[MaheshRavishankar]

Landing this for now, but happy to make changes after the fact

[dcaballe]

I'd have liked to have more time to review this before landing. This is a pretty low level and generic peephole. At least, we should move this to a more generic and later place in the pipeline to make sure that: 1) this also triggers for any potential matches outside of the emulate narrow pass, 2) all the potential matches have been generated (i.e., some high-level vector ops have been lowered) and, 3) these patterns can be extended to support more cases as needed (instead of duplicate the rewrites again for cases outside of the emulate narrow pass). Do you think you could take care of this?

[MaheshRavishankar]

I'd have liked to have more time to review this before landing.

Understood. I didnt think there were major issues here, and these are patterns that are know to work well for all backends. I am not actually able to put a finger on the issue that you see here really. So maybe I didnt fully understand the concerns here.
em

This is a pretty low level and generic peephole. At least, we should move this to a more generic and later place in the pipeline to make sure that: 1) this also triggers for any potential matches outside of the emulate narrow pass, 2) all the potential matches have been generated (i.e., some high-level vector ops have been lowered) and, 3) these patterns can be extended to support more cases as needed (instead of duplicate the rewrites again for cases outside of the emulate narrow pass). Do you think you could take care of this?

I am not sure I follow this either.... These should trigger only after the emulate pass inserts the bitcasts needed to match the vector types. Could you clarify a bit more what (2) and (3) are?

[dcaballe]

I am not sure I follow this either.... These should trigger only after the emulate pass inserts the bitcasts needed to match the vector types. Could you clarify a bit more what (2) and (3) are?

2. This assumes that all the bitcast -> extui targeted by this peephole are generated by this pass. My point is that these ops can be generated by any other pass in MLIR now or in the future.

3. Patterns can (and probably will) be extended to support more combinations of types and they will grow, making micro changes #2 even more likely to happen.

In other words, this is a general peephole optimization so let's putting in a place where it can be genetically applied and extended outside of EmulateNarrowTypes.

[MaheshRavishankar]

I am not sure I follow this either.... These should trigger only after the emulate pass inserts the bitcasts needed to match the vector types. Could you clarify a bit more what (2) and (3) are?

2. This assumes that all the bitcast -> extui targeted by this peephole are generated by this pass. My point is that these ops can be generated by any other pass in MLIR now or in the future.

3. Patterns can (and probably will) be extended to support more combinations of types and they will grow, making micro changes #2 even more likely to happen.

In other words, this is a general peephole optimization so let's putting in a place where it can be genetically applied and extended outside of EmulateNarrowTypes.

Sure maybe, but for now keeping related things together. If there is a use for this outside of this pass, we can move it to a separate pass. Adding a new pass just for these patterns seems unnecessary right now