[PATCH] D75743: [GlobalISel] fix crash in narrowScalarExtract if DstRegs only has one register

[Dominik Montada via Phabricator via llvm-commits]

gargaroff added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3688-3694
   Register DstReg = MI.getOperand(0).getReg();
-  if(MRI.getType(DstReg).isVector())
+  if (MRI.getType(DstReg).isVector())
     MIRBuilder.buildBuildVector(DstReg, DstRegs);
-  else
+  else if (DstRegs.size() > 1)
     MIRBuilder.buildMerge(DstReg, DstRegs);
+  else
+    MIRBuilder.buildCopy(DstReg, DstRegs[0]);
----------------
arsenm wrote:
> Could this just write the original result into DstReg and avoid the copy?
Hm, the only thing we could reasonably do here to avoid the copy is to change the operand of the defining instruction of `DstRegs[0]`. The code for this would look something like this:

```
  else {
    MachineInstr *DefInst = MRI.getVRegDef(DstRegs[0]);
    assert(DefInst && "DstRegs[0] should have been defined");

    unsigned Idx = DefInst->getOpcode() == TargetOpcode::G_UNMERGE_VALUES ? OpStart / NarrowSize : 0;

    DefInst->getOperand(Idx).setReg(DstReg);
  }
```

I mean it avoids the copy I guess, but not sure if that is really better here?

I also tried to rewrite the code to just use `DstReg` where we want to have it in the first place, but that just clutters the code with if-checks, so also not really nice.

Let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75743