llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.78k stars 11.9k forks source link

Document GlobalISel's combiner guidelines #92309

Open aemerson opened 5 months ago

aemerson commented 5 months ago

@tschuett made a good point that our approach to writing combiners in GlobalISel needs some documentation here: https://github.com/llvm/llvm-project/pull/91922

This issue is to track that initial work on elaborating things like:

tschuett commented 5 months ago

I vote for a section about the combiner design. We compile the Combine.td into a matchable. 10 more lines.

Complex logic is delegated to CombinerHelper Helper.

From this design, we can conclude:

tschuett commented 3 months ago

We are not rebuilding InstCombine. We take inspirations from InstCombine. If there are semantic uncertainties, we take InstCombine as the reference.

tschuett commented 3 months ago

Assuming the combiner is at least an artefact combiner, the AArch64 post legalization combiner is too weak, see https://github.com/llvm/llvm-project/pull/101327#issuecomment-2263341015

tschuett commented 3 months ago

We should/must combine ADDEs post legalization, but we don't.

tschuett commented 1 month ago

To make a secret public, I have some interest in https://reviews.llvm.org/D96031. If this is a legacy way of doing combines and the rules changed, we should document that and make the rules public.

https://github.com/llvm/llvm-project/pull/90618 is plain port from InstCombine, which I believe was a great idea and big win for us.