Issue 128843
Summary GlobalISel inserts illegal COPY operation
Labels backend:AArch64, llvm:globalisel
Assignees
Reporter sdesmalen-arm
    ### Reproducer:
```
; compile with `llc -mattr=+fullfp16 -O0`

define i32 @reproducer() {
  %retval = call i32 @llvm.aarch64.neon.fcvtzs.i32.f16(half 0xH4566)
  ret i32 %retval
}
```

This runs into a llvm_unreachable:

```
H0 = COPY W0
unimplemented reg-to-reg copy

UNREACHABLE executed at llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5250
```

This issue may be related to #79822.

### My Analysis:

GlobalISel seems to assume that everything goes by default into a GPR, because after RegBankSelect the IR looks like this:
```
bb.1 (%ir-block.0):
  %1:fpr(s16) = G_FCONSTANT half 0xH4566
  %2:gpr(s16) = COPY %1:fpr(s16)
  %0:gpr(s32) = G_INTRINSIC intrinsic(@llvm.aarch64.neon.fcvtzs), %2:gpr(s16)
```

(1) During the 'instruction selection' step for `G_INTRINSIC` it inserts a COPY to move `%2` from a GPR -> FPR. This results in:
```
bb.1 (%ir-block.0):
  %1:fpr(s16) = G_FCONSTANT half 0xH4566
  %2:gpr(s16) = COPY %1:fpr(s16)      
 %3:fpr(s16) = COPY %2:gpr(s16)      // COPY inserted by lowering of G_INTRINSIC
  %0:gpr(s32) = FCVTZSUWHr %3
```

(2) During the 'instruction selection' step for `%2:gpr(s16) = COPY %1:fpr(s16)` it:
* Legalises the operation, e.g. if the size doesn't match, it either copies the sub-register or it inserts it into a larger register using SUBREG_TO_REG.
* Legalises the type (selects the appropriate register class for the result)

After this, the IR looks like:
```
  %6:fpr(s16) = G_FCONSTANT half 0xH4566
  %4:fpr32 = SUBREG_TO_REG 0, %6:fpr16, %subreg.hsub
 %2:gpr32all = COPY %4:fpr32         // type is promoted, and SUBREG_TO_REG is inserted
  %3:fpr16 = COPY %2:gpr32all
  %0:gpr32 = FCVTZSUWHr %3:fpr16
```

Because the GlobalISel instruction selector has explicit code not to revisit the `%3:fpr16 = COPY %2:gpr32all`, the COPY remains in place.
```
// Snippet from InstructionSelect::selectMachineFunction

for (auto End = MBB->rend(); MIIMaintainer.MII != End;) {
  MachineInstr &MI = *MIIMaintainer.MII;
  // Increment early to skip instructions inserted by select().
  ++MIIMaintainer.MII;
```

I think this COPY should instead have been turned into an FMOV by SelectionDAG, rather than leaving it a COPY and hoping the target will implement it. My understanding is that `copyPhysReg` should only implement copies where the sizes match.

I don't understand GlobalISel enough to make a judgement call on how to fix this though, @aemerson any ideas?
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs

Reply via email to