[PATCH] D61289: [globalisel] Add G_SEXT_INREG

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Is the intention to now use this instead of the current system of relying on the cleanup of legalization artifacts?



================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:119
 
 class SrcOp {
   union {
----------------
I think the SrcOp changes should be split to a separate patch


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:715-718
+          MIRBuilder
+              .buildInstr(TargetOpcode::G_TRUNC, {NarrowTy}, {MO1.getReg()})
+              ->getOperand(0)
+              .getReg());
----------------
This wrapping is really weird looking. There should probably be a dedicated buildTrunc which would help some


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1708-1709
+  case TargetOpcode::G_SEXT_INREG: {
+    if (!MI.getOperand(2).isImm())
+      return UnableToLegalize;
+    int64_t SizeInBits = MI.getOperand(2).getImm();
----------------
This should be illegal and caught by the verifier? This shouldn't need to check for illegal cases


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:1328
+    break;
+  }
   default:
----------------
Should check for matching vector elements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61289