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

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/docs/GlobalISel/MIRPatterns.rst:189-192
+  def Foo : GICombineRule<
+    (defs root:$dst),
+    (match (G_FOO $tmp, $src), (G_BAR $dst, $tmp)),
+    (apply (G_IMPLICIT_DEF $dst))>;
----------------
aemerson wrote:
> It doesn't have to be in the initial patch, but can this be addressed later? If we migrate to using MIR patterns more often, this seems wasteful (generating a new inst, observer call, CSE, DCE)
> 
> Similar thing for the replacing a register case, it would be good to have a way to trigger a `MRI.replaceRegWith()`.
Sure, I'm leaning towards adding more `apply` magic operators so we can do things like `(apply (erase $d), (replace $z, $x)`) but I haven't looked into it yet. I'd do that in a follow up patch.

It's one of the things I want to look at alongside intrinsics matching support (+ migrating some combiner rules of course)


================
Comment at: llvm/utils/TableGen/GlobalISelCombinerMatchTableEmitter.cpp:677-678
+
+      if (!Operand.isDef())
+        continue;
+
----------------
aemerson wrote:
> can move this above the definition of `Def`
No, we always need to create an entry for any operand in the OperandTable. It's how we can differentiate "this is a live-in" from "this doesn't exist". I'll make it clearer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156315