[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:87-89
+* Immediates always emit a CImm in the ``apply`` pattern of a
+  ``GICombineRule``. There is no way to implicitly emit a ``G_CONSTANT``,
+  or emit an simple immediate operand.
----------------
Pierre-vh wrote:
> arsenm wrote:
> > This is unusably broken? You can't produce a valid G_* instruction with a cimm operand
> `G_CONSTANT` uses a CImm, AFAIK
> See `MachineIRBuilder::buildConstant` with `addCImm` uses.
> 
> Do you have example of instructions that need a direct imm? I can test them and fix them.
> 
> I'm also wondering if I should just go ahead and make immediates always emit G_CONSTANT except in specific cases (currently only G_CONSTANT I think?). It'd be a usability improvement.
addImm uses correspond to immarg arguments. Everything else uses G_CONSTANT with a isCImm argument as a materialized constant


================
Comment at: llvm/docs/GlobalISel/MIRPatterns.rst:109
+    (match (G_MUL $dst, $x, 1)),
+    (apply (COPY $dst, $x))
+  >;
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156315