[GlobalIsel] Add Gallery to MIR Patterns

[tschuett]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

Author: Thorsten Schütt (tschuett)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/89974.diff

1 Files Affected:

• (modified) llvm/docs/GlobalISel/MIRPatterns.rst (+18)

diff --git a/llvm/docs/GlobalISel/MIRPatterns.rst b/llvm/docs/GlobalISel/MIRPatterns.rst
index 728e3247014452..97717005ebd619 100644
--- a/llvm/docs/GlobalISel/MIRPatterns.rst
+++ b/llvm/docs/GlobalISel/MIRPatterns.rst
@@ -514,3 +514,21 @@ of operands.
     (match (does_not_bind $tmp, $x)
            (G_MUL $dst, $x, $tmp)),
     (apply (COPY $dst, $x))>;
+
+
+ Gallery
+----------------
+
+We should use precise patterns that state our intentions. Please avoid
+using wip_match_opcode in patterns.
+
+.. code-block:: text
+  :caption: Example fold ext(trunc)
+
+(match (wip_match_opcode G_ZEXT):$root,
+
+(match (G_TRUNC $src, $x),
+       (G_ZEXT $root, $src),
+
+(match (G_TRUNC $src, $x, (MIFlags NoUWrap)),
+       (G_ZEXT $root, $src),

[arsenm]

Probably should include the apply part of the example?

[arsenm]

Not sure why this is showing 2 nearly identical patterns. What is the MIFlags doing if the first pattern didn't need flag checks?

[tschuett]

The third pattern is more precise. The one above hits with no flags and with NoSWrap.

[arsenm]

It's more precise, but if the first pattern works you wouldn't write the second

[tschuett]

There is a lot of misbehaviour in Combine.td. I just want some examples that we can point to in review.

[jayfoad]

I think it's a good idea to have some examples. I'd like it even more if each one had a comment explaining what it matches, either in English or in some pseudocode like Match (zext (trunc x)).

[tschuett]

At the top it states: fold ext(trunk:nuw)
I want to prevent misbehaviour like:

llvm-project/llvm/include/llvm/Target/GlobalISel/Combine.td

Line 722 in fc538b0

// Fold (zext (trunc x)) -> x if the source type is same as the destination type

[jayfoad]

What is the misbehaviour?

[tschuett]

It registers on G_ZEXT:

(match (wip_match_opcode G_ZEXT):$root,

but it states as a comment that it wants to fold zext(trunk(x).

[jayfoad]

What's wrong with that? wip_match_opcode just matches the "topmost" / "outermost" opcode, which is zext in this case. The C++ code looks for the inner node, trunc.

[tschuett]

It will match any G_ZEXT in the function. In most cases, there is no trunk on the zext. It is a waste of compile-time. If we would write more precise matches, it might reduce compile-time.

wip_match_opcode should be forbidden. Please write precise patterns.

[jayfoad]

OK. I was confused because "misbehaviour" generally implies that something is incorrect, not just suboptimal.

I would prefer comments on the individual patterns, e.g.:

// Imprecise: matches any G_ZEXT
...
// Better: matches G_ZEXT of G_TRUNC
...
// Precise: matches G_ZEXT of G_TRUNC with nuw flag
...

Without that, it is not clear to me why you have presented a list of similar-but-different patterns.

[jayfoad]

Suggested change

	:caption: Example fold ext(trunc:nuw)
	:caption: Example fold zext(trunc:nuw)

[jayfoad]

The text LGTM, but when I try to build docs-llvm-html I get:

Warning, treated as error:
docs/GlobalISel/MIRPatterns.rst:471:Error in "code-block" directive:
maximum 1 argument(s) allowed, 7 supplied.