[PATCH] D61289: [globalisel] Add G_SEXT_INREG

[Daniel Sanders via Phabricator via llvm-commits]

dsanders marked 24 inline comments as done.
dsanders added a comment.

The next update fixes the last couple nits that were left open after the LGTM. I'm planning to commit this tomorrow



================
Comment at: llvm/include/llvm/Target/GenericOpcodes.td:46
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type0:$src, untyped_imm_0:$sz);
+  let hasSideEffects = 0;
----------------
arsenm wrote:
> dsanders wrote:
> > arsenm wrote:
> > > dsanders wrote:
> > > > arsenm wrote:
> > > > > Are tablegen emitter changes needed for this? I was trying to add an immediate operand in D64054, which seemed to not work correctly
> > > > You mean the untyped_imm_0? It's needed to drive the verifier that Diana requested and tells it which immediate checks it needs to look for
> > > Yes. As far as I can tell the emitter will try looking for G_CONSTANT defined register for this. I agree there should be a special immediate operand for this, but I don't think this will work in a tablegen pattern as-is. The current form has a ValueType leaf, so matching the immediate wouldn't be quite the same.
> > I'm not sure I'm following. InOperandList in a GenericInstruction specifies the type constraints and uses special TypeOperand subclasses. They don't factor into tablegen patterns except in so far as requiring that types match.
> I mean I believe there's no way to define a node that will be capable of matching this instruction
Ah I see. Yes, we'll need to map sext_inreg to G_SEXT_INREG and add some special handling to convert the MVT operand to an immediate.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:770
+                                          {PartialExtensionReg, AshrCstReg})
+                              ->getOperand(0)
+                              .getReg());
----------------
Petar.Avramovic wrote:
> `getOperand(0).getReg()` -> `getReg(0)`
I tried this change but it resulted in errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61289