[AArch64][GlobalISel] When copying from a gpr32 to an fpr16 reg, convert to fpr32 first

[aemerson]

Summary

This is a follow on commit to D43310 where we fix the other direction of copy.

For this case, after converting the source from gpr32 -> fpr32, we use a subregister copy, which is essentially what EXTRACT_SUBREG does in SDAG land.

The codegen isn't great at -O0, but I'm not sure what else we can do.

[aemerson]

As stated in PR36345 I'm committing this now to get it into 6.0.

[aemerson]

r325550.

[qcolombet]

lib/Target/AArch64/AArch64InstructionSelector.cpp

330

At this point of the pipeline we usually don't write copy from subregs directly.
Could you use EXTRACT_SUBREG instead?

[aemerson]

lib/Target/AArch64/AArch64InstructionSelector.cpp

330

That’s what I tried initially, but ExpandPseudos fell over because it expects all EXTRACT_SUBREGs to be eliminated by then. Apparently EXTRACT_SUBREG is normally converted to copies in InstrEmitter during SDISel, so the rest of the pipeline isn’t prepared to deal with it.

lib/Target/AArch64/AArch64InstructionSelector.cpp

case AArch64::FPRRegBankID:
     case 64:
       return isStore ? AArch64::STRDui : AArch64::LDRDui;
     }
     break;
   }
   return GenericOpc;
 }
 
+static bool selectFP16CopyFromGPR32(MachineInstr &I, const TargetInstrInfo &TII,
+                                    MachineRegisterInfo &MRI, unsigned SrcReg) {
+  // Copies from gpr32 to fpr16 need to use a sub-register copy.
+  unsigned CopyReg = MRI.createVirtualRegister(&AArch64::FPR32RegClass);
+  BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(AArch64::COPY))
+      .addDef(CopyReg)
+      .addUse(SrcReg);
+  unsigned SubRegCopy = MRI.createVirtualRegister(&AArch64::FPR16RegClass);
+  BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(TargetOpcode::COPY))
+      .addDef(SubRegCopy)
+      .addUse(CopyReg, 0, AArch64::hsub);
+
+  MachineOperand &RegOp = I.getOperand(1);
+  RegOp.setReg(SubRegCopy);
+  return true;
+}
+
 static bool selectCopy(MachineInstr &I, const TargetInstrInfo &TII,
                        MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
                        const RegisterBankInfo &RBI) {
 
   unsigned DstReg = I.getOperand(0).getReg();
+  unsigned SrcReg = I.getOperand(1).getReg();
+
   if (TargetRegisterInfo::isPhysicalRegister(DstReg)) {
+    if (TRI.getRegClass(AArch64::FPR16RegClassID)->contains(DstReg) &&
+        !TargetRegisterInfo::isPhysicalRegister(SrcReg)) {
+      const RegisterBank &RegBank = *RBI.getRegBank(SrcReg, MRI, TRI);
+      const TargetRegisterClass *SrcRC = getRegClassForTypeOnBank(
+          MRI.getType(SrcReg), RegBank, RBI, /* GetAllRegSet */ true);
+      if (SrcRC == &AArch64::GPR32allRegClass)
+        return selectFP16CopyFromGPR32(I, TII, MRI, SrcReg);
+    }
     assert(I.isCopy() && "Generic operators do not allow physical registers");
     return true;
   }
 
   const RegisterBank &RegBank = *RBI.getRegBank(DstReg, MRI, TRI);
   const unsigned DstSize = MRI.getType(DstReg).getSizeInBits();
   (void)DstSize;
-  unsigned SrcReg = I.getOperand(1).getReg();
   const unsigned SrcSize = RBI.getSizeInBits(SrcReg, MRI, TRI);
   (void)SrcSize;
   assert((!TargetRegisterInfo::isPhysicalRegister(SrcReg) || I.isCopy()) &&
          "No phys reg on generic operators");
   assert(
       (DstSize == SrcSize ||
        // Copies are a mean to setup initial types, the number of
        // bits may not exactly match.
static bool selectCopy(MachineInstr &I, const TargetInstrInfo &TII,
   const TargetRegisterClass *RC = getRegClassForTypeOnBank(
       MRI.getType(DstReg), RegBank, RBI, /* GetAllRegSet */ true);
   if (!RC) {
     DEBUG(dbgs() << "Unexpected bitcast size " << DstSize << '\n');
     return false;
   }
 
   if (!TargetRegisterInfo::isPhysicalRegister(SrcReg)) {
-    const RegClassOrRegBank &RegClassOrBank =
-      MRI.getRegClassOrRegBank(SrcReg);
-
+    const RegClassOrRegBank &RegClassOrBank = MRI.getRegClassOrRegBank(SrcReg);
     const TargetRegisterClass *SrcRC =
         RegClassOrBank.dyn_cast<const TargetRegisterClass *>();
     const RegisterBank *RB = nullptr;
     if (!SrcRC) {
       RB = RegClassOrBank.get<const RegisterBank *>();
       SrcRC = getRegClassForTypeOnBank(MRI.getType(SrcReg), *RB, RBI, true);
     }
     // Copies from fpr16 to gpr32 need to use SUBREG_TO_REG.
     if (RC == &AArch64::GPR32allRegClass && SrcRC == &AArch64::FPR16RegClass) {
       unsigned PromoteReg = MRI.createVirtualRegister(&AArch64::FPR32RegClass);
       BuildMI(*I.getParent(), I, I.getDebugLoc(),
               TII.get(AArch64::SUBREG_TO_REG))
           .addDef(PromoteReg)
           .addImm(0)
           .addUse(SrcReg)
           .addImm(AArch64::hsub);
       MachineOperand &RegOp = I.getOperand(1);
       RegOp.setReg(PromoteReg);
+    } else if (RC == &AArch64::FPR16RegClass &&
+               SrcRC == &AArch64::GPR32allRegClass) {
+      selectFP16CopyFromGPR32(I, TII, MRI, SrcReg);
     }
   }
 
   // No need to constrain SrcReg. It will get constrained when
   // we hit another of its use or its defs.
   // Copies do not have constraints.
   if (!RBI.constrainGenericRegister(DstReg, *RC, MRI)) {
     DEBUG(dbgs() << "Failed to constrain " << TII.getName(I.getOpcode())

test/CodeGen/AArch64/GlobalISel/fp16-copy-gpr.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -mtriple=aarch64-unknown-unknown -o - -global-isel -verify-machineinstrs -run-pass=instruction-select %s | FileCheck %s
 
 # PR36345
 --- |
   target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
   target triple = "aarch64-arm-none-eabi"
 
-  %struct.struct2 = type { [2 x half] }
+  ; Function Attrs: noinline nounwind optnone
+  define void @fp16_to_gpr([2 x half], [2 x half]* %addr) {
+    ret void
+  }
 
-  @global_arg0 = common dso_local global %struct.struct2 zeroinitializer, align 2
+  define void @gpr_to_fp16() {
+    ret void
+  }
 
-  ; Function Attrs: noinline nounwind optnone
-  define dso_local void @c_test([2 x half], [2 x half]* %addr) {
-    store [2 x half] %0, [2 x half]* %addr, align 2
+  define void @gpr_to_fp16_physreg() {
     ret void
   }
 ...
 ---
-name:            c_test
+name:            fp16_to_gpr
 alignment:       2
 legalized:       true
 regBankSelected: true
 tracksRegLiveness: true
 registers:
   - { id: 0, class: gpr }
   - { id: 1, class: fpr }
   - { id: 2, class: fpr }
   - { id: 3, class: gpr }
   - { id: 4, class: gpr }
   - { id: 5, class: gpr }
   - { id: 6, class: gpr }
   - { id: 7, class: gpr }
   - { id: 8, class: gpr }
   - { id: 9, class: gpr }
   - { id: 10, class: gpr }
   - { id: 11, class: gpr }
   - { id: 12, class: gpr }
 body:             |
   bb.1 (%ir-block.1):
     liveins: $h0, $h1, $x0
 
-    ; CHECK-LABEL: name: c_test
+    ; CHECK-LABEL: name: fp16_to_gpr
     ; CHECK: liveins: $h0, $h1, $x0
     ; CHECK: [[COPY:%[0-9]+]]:fpr16 = COPY $h0
     ; CHECK: [[COPY1:%[0-9]+]]:fpr16 = COPY $h1
     ; CHECK: [[DEF:%[0-9]+]]:gpr32 = IMPLICIT_DEF
     ; CHECK: [[SUBREG_TO_REG:%[0-9]+]]:fpr32 = SUBREG_TO_REG 0, [[COPY]], %subreg.hsub
     ; CHECK: [[COPY2:%[0-9]+]]:gpr32 = COPY [[SUBREG_TO_REG]]
     ; CHECK: [[BFMWri:%[0-9]+]]:gpr32 = BFMWri [[DEF]], [[COPY2]], 0, 15
     ; CHECK: [[SUBREG_TO_REG1:%[0-9]+]]:fpr32 = SUBREG_TO_REG 0, [[COPY1]], %subreg.hsub
bb.1 (%ir-block.1):
     %12:gpr(s16) = COPY %2(s16)
     %5:gpr(s32) = G_INSERT %4, %12(s16), 16
     %0:gpr(s32) = COPY %5(s32)
     %6:gpr(p0) = COPY $x0
     G_STORE %0(s32), %6(p0) :: (store 4 into %ir.addr, align 2)
     RET_ReallyLR
 
 ...
+
+---
+name:            gpr_to_fp16
+alignment:       2
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+  - { id: 2, class: fpr }
+body:             |
+  bb.1 (%ir-block.0):
+    liveins: $w0
+
+    ; CHECK-LABEL: name: gpr_to_fp16
+    ; CHECK: liveins: $w0
+    ; CHECK: [[COPY:%[0-9]+]]:gpr32 = COPY $w0
+    ; CHECK: [[COPY1:%[0-9]+]]:fpr32 = COPY [[COPY]]
+    ; CHECK: [[COPY2:%[0-9]+]]:fpr16 = COPY [[COPY1]].hsub
+    ; CHECK: $h0 = COPY [[COPY2]]
+    ; CHECK: RET_ReallyLR implicit $h0
+    %0:gpr(s32) = COPY $w0
+    %1:gpr(s16) = G_TRUNC %0(s32)
+    %2:fpr(s16) = COPY %1(s16)
+    $h0 = COPY %2(s16)
+    RET_ReallyLR implicit $h0
+
+...
+---
+name:            gpr_to_fp16_physreg
+alignment:       2
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+body:             |
+  bb.1 (%ir-block.0):
+    liveins: $w0
+
+    ; CHECK-LABEL: name: gpr_to_fp16_physreg
+    ; CHECK: liveins: $w0
+    ; CHECK: [[COPY:%[0-9]+]]:gpr32 = COPY $w0
+    ; CHECK: [[COPY1:%[0-9]+]]:fpr32 = COPY [[COPY]]
+    ; CHECK: [[COPY2:%[0-9]+]]:fpr16 = COPY [[COPY1]].hsub
+    ; CHECK: $h0 = COPY [[COPY2]]
+    ; CHECK: RET_ReallyLR implicit $h0
+    %0:gpr(s32) = COPY $w0
+    %1:gpr(s16) = G_TRUNC %0(s32)
+    $h0 = COPY %1(s16)
+    RET_ReallyLR implicit $h0
+
+...