[PATCH] D156315: [RFC][GlobalISel] Overhauled MIR Patterns Support for Combiners

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/docs/GlobalISel/MIRPatterns.rst:109
+    (match (G_MUL $dst, $x, 1)),
+    (apply (COPY $dst, $x))
+  >;
----------------
Pierre-vh wrote:
> arsenm wrote:
> > Pierre-vh wrote:
> > > arsenm wrote:
> > > > maybe mention somewhere that you need to copy. 
> > > > 
> > > > How are patterns that discard their outputs handled? Does it error on those?
> > > > How are patterns that discard their outputs handled? Does it error on those?
> > > 
> > > Good question, I think it will indeed error out.
> > > Do you mean that there's patterns that just delete instructions? I could add a special case for `(apply)` to just erase the root. Would that work for you?
> > We have some special case atomic optimizations that discard the result if use_empty, it's currently dealt with in selection but theoretically could be a combine
> If I understand correctly it'd look like this?
> ```
> (match (G_ATOMIC_FOO $x, $y), "return MRI.use_empty(${x}.getReg())"))
> (apply) // somehow delete it
> ```
> 
> Then yeah, that's not supported so far. I can add a special case to just delete the root if there are no apply patterns if you think that'd work?
> 
> 
> An easy workaround would be to just set $x to IMPLICIT_DEF, but if G_ATOMIC_FOO has no defs then there's no workaround other than using C++ in the (apply).
Yes the implicit_def thing should be good enough. It's not that important, but would be nice to document


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156315/new/

https://reviews.llvm.org/D156315