[GlobalISel|ARM] : Allow legalizing G_FSUB

[javed.absar]

Summary

Adding support for VSUB.

[rovka]

Hi Javed,

This is going in the right direction.

Your test is validating the whole GISel pipeline. You should add more specific tests in arm-legalize-fp.mir, arm-regbankselect.mir and arm-instruction-select.mir. That will make it very easy to spot what's missing.

[javed.absar]

Thanks Diana for the feedback. I added the missing tests and they now all pass.

[rovka]

LGTM, thanks!

test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir

346

Nit: These checks are redundant now, since the ones below also check the register classes on each def. You can remove them.

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

case TargetOpcode::G_SREM:
     assert(Size == 32 && "Unsupported size");
     return RTLIB::SREM_I32;
   case TargetOpcode::G_UREM:
     assert(Size == 32 && "Unsupported size");
     return RTLIB::UREM_I32;
   case TargetOpcode::G_FADD:
     assert((Size == 32 || Size == 64) && "Unsupported size");
     return Size == 64 ? RTLIB::ADD_F64 : RTLIB::ADD_F32;
+  case TargetOpcode::G_FSUB:
+    assert((Size == 32 || Size == 64) && "Unsupported size");
+    return Size == 64 ? RTLIB::SUB_F64 : RTLIB::SUB_F32;
   case TargetOpcode::G_FREM:
     return Size == 64 ? RTLIB::REM_F64 : RTLIB::REM_F32;
   case TargetOpcode::G_FPOW:
     return Size == 64 ? RTLIB::POW_F64 : RTLIB::POW_F32;
   }
   llvm_unreachable("Unknown libcall function");
 }
 
LegalizerHelper::libcall(MachineInstr &MI) {
   case TargetOpcode::G_UREM: {
     Type *HLTy = Type::getInt32Ty(Ctx);
     auto Status = simpleLibcall(MI, MIRBuilder, Size, HLTy);
     if (Status != Legalized)
       return Status;
     break;
   }
   case TargetOpcode::G_FADD:
+  case TargetOpcode::G_FSUB:
   case TargetOpcode::G_FPOW:
   case TargetOpcode::G_FREM: {
     Type *HLTy = Size == 64 ? Type::getDoubleTy(Ctx) : Type::getFloatTy(Ctx);
     auto Status = simpleLibcall(MI, MIRBuilder, Size, HLTy);
     if (Status != Legalized)
       return Status;
     break;
   }

lib/Target/ARM/ARMLegalizerInfo.cpp

ARMLegalizerInfo::ARMLegalizerInfo(const ARMSubtarget &ST) {
 
   setAction({G_ICMP, s1}, Legal);
   for (auto Ty : {s8, s16})
     setAction({G_ICMP, 1, Ty}, WidenScalar);
   for (auto Ty : {s32, p0})
     setAction({G_ICMP, 1, Ty}, Legal);
 
   if (!ST.useSoftFloat() && ST.hasVFP2()) {
-    setAction({G_FADD, s32}, Legal);
-    setAction({G_FADD, s64}, Legal);
+    for (unsigned BinOp : {G_FADD, G_FSUB})
+      for (auto Ty : {s32, s64})
+        setAction({BinOp, Ty}, Legal);
 
     setAction({G_LOAD, s64}, Legal);
     setAction({G_STORE, s64}, Legal);
 
     setAction({G_FCMP, s1}, Legal);
     setAction({G_FCMP, 1, s32}, Legal);
     setAction({G_FCMP, 1, s64}, Legal);
   } else {
+    for (unsigned BinOp : {G_FADD, G_FSUB})
       for (auto Ty : {s32, s64})
-      setAction({G_FADD, Ty}, Libcall);
+        setAction({BinOp, Ty}, Libcall);
 
     setAction({G_FCMP, s1}, Legal);
     setAction({G_FCMP, 1, s32}, Custom);
     setAction({G_FCMP, 1, s64}, Custom);
 
     if (AEABI(ST))
       setFCmpLibcallsAEABI();
     else

lib/Target/ARM/ARMRegisterBankInfo.cpp

case G_STORE: {
     LLT Ty = MRI.getType(MI.getOperand(0).getReg());
     OperandsMapping =
         Ty.getSizeInBits() == 64
             ? getOperandsMapping({&ARM::ValueMappings[ARM::DPR3OpsIdx],
                                   &ARM::ValueMappings[ARM::GPR3OpsIdx]})
             : &ARM::ValueMappings[ARM::GPR3OpsIdx];
     break;
   }
-  case G_FADD: {
+  case G_FADD:
+  case G_FSUB: {
     LLT Ty = MRI.getType(MI.getOperand(0).getReg());
-    assert((Ty.getSizeInBits() == 32 || Ty.getSizeInBits() == 64) &&
-           "Unsupported size for G_FADD");
-    OperandsMapping = Ty.getSizeInBits() == 64
+    OperandsMapping =Ty.getSizeInBits() == 64
                           ? &ARM::ValueMappings[ARM::DPR3OpsIdx]
                           : &ARM::ValueMappings[ARM::SPR3OpsIdx];
     break;
   }
   case G_CONSTANT:
   case G_FRAME_INDEX:
   case G_GLOBAL_VALUE:
     OperandsMapping =

test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir

 # RUN: llc -O0 -mtriple arm-- -global-isel -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
 --- |
   define void @test_trunc_and_zext_s1() { ret void }
   define void @test_trunc_and_sext_s1() { ret void }
   define void @test_trunc_and_sext_s8() { ret void }
   define void @test_trunc_and_zext_s16() { ret void }
   define void @test_trunc_and_anyext_s8() { ret void }
   define void @test_trunc_and_anyext_s16() { ret void }
 
   define void @test_add_s32() { ret void }
 
   define void @test_fadd_s32() #0 { ret void }
   define void @test_fadd_s64() #0 { ret void }
 
+  define void @test_fsub_s32() #0 { ret void }
+  define void @test_fsub_s64() #0 { ret void }
+
   define void @test_sub_s32() { ret void }
 
   define void @test_mul_s32() #1 { ret void }
   define void @test_mulv5_s32() { ret void }
 
   define void @test_sdiv_s32() #2 { ret void }
   define void @test_udiv_s32() #2 { ret void }
 
bb.0:
 
     %d0 = COPY %2(s64)
     ; CHECK: %d0 = COPY [[VREGSUM]]
 
     BX_RET 14, _, implicit %d0
     ; CHECK: BX_RET 14, _, implicit %d0
 ...
 ---
+name:            test_fsub_s32
+# CHECK-LABEL: name: test_fsub_s32
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: fprb }
+  - { id: 2, class: fprb }
+# CHECK: id: 0, class: spr
+# CHECK: id: 1, class: spr
+# CHECK: id: 2, class: spr
+body:             |
+  bb.0:
+    liveins: %s0, %s1
+
+    %0(s32) = COPY %s0
+    ; CHECK: [[VREGX:%[0-9]+]]:spr = COPY %s0
+
+    %1(s32) = COPY %s1
+    ; CHECK: [[VREGY:%[0-9]+]]:spr = COPY %s1
+
+    %2(s32) = G_FSUB %0, %1
+    ; CHECK: [[VREGSUM:%[0-9]+]]:spr = VSUBS [[VREGX]], [[VREGY]], 14, _
+
+    %s0 = COPY %2(s32)
+    ; CHECK: %s0 = COPY [[VREGSUM]]
+
+    BX_RET 14, _, implicit %s0
+    ; CHECK: BX_RET 14, _, implicit %s0
+...
+---
+name:            test_fsub_s64
+# CHECK-LABEL: name: test_fsub_s64
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: fprb }
+  - { id: 2, class: fprb }
+# CHECK: id: 0, class: dpr
+# CHECK: id: 1, class: dpr
+# CHECK: id: 2, class: dpr
+body:             |
+  bb.0:
+    liveins: %d0, %d1
+
+    %0(s64) = COPY %d0
+    ; CHECK: [[VREGX:%[0-9]+]]:dpr = COPY %d0
+
+    %1(s64) = COPY %d1
+    ; CHECK: [[VREGY:%[0-9]+]]:dpr = COPY %d1
+
+    %2(s64) = G_FSUB %0, %1
+    ; CHECK: [[VREGSUM:%[0-9]+]]:dpr = VSUBD [[VREGX]], [[VREGY]], 14, _
+
+    %d0 = COPY %2(s64)
+    ; CHECK: %d0 = COPY [[VREGSUM]]
+
+    BX_RET 14, _, implicit %d0
+    ; CHECK: BX_RET 14, _, implicit %d0
+...
+---
 name:            test_sub_s32
 # CHECK-LABEL: name: test_sub_s32
 legalized:       true
 regBankSelected: true
 selected:        false
 # CHECK: selected: true
 registers:
   - { id: 0, class: gprb }

test/CodeGen/ARM/GlobalISel/arm-isel-fp.ll

 ; CHECK-LABEL: test_add_double:
 ; HARD: vadd.f64
 ; SOFT-AEABI: bl __aeabi_dadd
 ; SOFT-DEFAULT: bl __adddf3
   %r = fadd double %x, %y
   ret double %r
 }
 
+define arm_aapcscc float @test_sub_float(float %x, float %y) {
+; CHECK-LABEL: test_sub_float:
+; HARD: vsub.f32
+; SOFT-AEABI: bl __aeabi_fsub
+; SOFT-DEFAULT: bl __subsf3
+  %r = fsub float %x, %y
+  ret float %r
+}
+
+define arm_aapcscc double @test_sub_double(double %x, double %y) {
+; CHECK-LABEL: test_sub_double:
+; HARD: vsub.f64
+; SOFT-AEABI: bl __aeabi_dsub
+; SOFT-DEFAULT: bl __subdf3
+  %r = fsub double %x, %y
+  ret double %r
+}
 define arm_aapcs_vfpcc i32 @test_cmp_float_ogt(float %x, float %y) {
 ; CHECK-LABEL: test_cmp_float_ogt
 ; HARD: vcmp.f32
 ; HARD: vmrs APSR_nzcv, fpscr
 ; HARD-NEXT: movgt
 ; SOFT-AEABI: bl __aeabi_fcmpgt
 ; SOFT-DEFAULT: bl __gtsf2
 entry:

test/CodeGen/ARM/GlobalISel/arm-legalize-fp.mir

 # RUN: llc -mtriple arm-linux-gnueabihf -mattr=+vfp2 -float-abi=hard -global-isel -run-pass=legalizer %s -o - | FileCheck %s -check-prefix CHECK -check-prefix HARD
 # RUN: llc -mtriple arm-linux-gnueabi -mattr=+vfp2,+soft-float -float-abi=soft -global-isel -run-pass=legalizer %s -o - | FileCheck %s -check-prefix CHECK -check-prefix SOFT -check-prefix SOFT-AEABI
 # RUN: llc -mtriple arm-linux-gnu -mattr=+soft-float -float-abi=soft -global-isel -run-pass=legalizer %s -o - | FileCheck %s  -check-prefix CHECK -check-prefix SOFT -check-prefix SOFT-DEFAULT
 --- |
   define void @test_frem_float() { ret void }
   define void @test_frem_double() { ret void }
 
   define void @test_fpow_float() { ret void }
   define void @test_fpow_double() { ret void }
 
   define void @test_fadd_float() { ret void }
   define void @test_fadd_double() { ret void }
 
+  define void @test_fsub_float() { ret void }
+  define void @test_fsub_double() { ret void }
+
   define void @test_fcmp_true_s32() { ret void }
   define void @test_fcmp_false_s32() { ret void }
 
   define void @test_fcmp_oeq_s32() { ret void }
   define void @test_fcmp_ogt_s32() { ret void }
   define void @test_fcmp_oge_s32() { ret void }
   define void @test_fcmp_olt_s32() { ret void }
   define void @test_fcmp_ole_s32() { ret void }
bb.0:
     %6(s64) = G_FADD %4, %5
     ; HARD-DAG: G_UNMERGE_VALUES [[R]](s64)
     %7(s32),%8(s32) = G_UNMERGE_VALUES %6(s64)
     %r0 = COPY %7(s32)
     %r1 = COPY %8(s32)
     BX_RET 14, _, implicit %r0, implicit %r1
 ...
 ---
+name:            test_fsub_float
+# CHECK-LABEL: name: test_fsub_float
+legalized:       false
+# CHECK: legalized: true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    ; CHECK-DAG: [[X:%[0-9]+]]:_(s32) = COPY %r0
+    ; CHECK-DAG: [[Y:%[0-9]+]]:_(s32) = COPY %r1
+    %0(s32) = COPY %r0
+    %1(s32) = COPY %r1
+    ; HARD: [[R:%[0-9]+]]:_(s32) = G_FSUB [[X]], [[Y]]
+    ; SOFT-NOT: G_FSUB
+    ; SOFT: ADJCALLSTACKDOWN
+    ; SOFT-DAG: %r0 = COPY [[X]]
+    ; SOFT-DAG: %r1 = COPY [[Y]]
+    ; SOFT-AEABI: BL $__aeabi_fsub, {{.*}}, implicit %r0, implicit %r1, implicit-def %r0
+    ; SOFT-DEFAULT: BL $__subsf3, {{.*}}, implicit %r0, implicit %r1, implicit-def %r0
+    ; SOFT: [[R:%[0-9]+]]:_(s32) = COPY %r0
+    ; SOFT: ADJCALLSTACKUP
+    ; SOFT-NOT: G_FSUB
+    %2(s32) = G_FSUB %0, %1
+    ; CHECK: %r0 = COPY [[R]]
+    %r0 = COPY %2(s32)
+    BX_RET 14, _, implicit %r0
+...
+---
+name:            test_fsub_double
+# CHECK-LABEL: name: test_fsub_double
+legalized:       false
+# CHECK: legalized: true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+  - { id: 3, class: _ }
+  - { id: 4, class: _ }
+  - { id: 5, class: _ }
+  - { id: 6, class: _ }
+  - { id: 7, class: _ }
+  - { id: 8, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %r1, %r2, %r3
+
+    ; CHECK-DAG: [[X0:%[0-9]+]]:_(s32) = COPY %r0
+    ; CHECK-DAG: [[X1:%[0-9]+]]:_(s32) = COPY %r1
+    ; CHECK-DAG: [[Y0:%[0-9]+]]:_(s32) = COPY %r2
+    ; CHECK-DAG: [[Y1:%[0-9]+]]:_(s32) = COPY %r3
+    %0(s32) = COPY %r0
+    %1(s32) = COPY %r1
+    %2(s32) = COPY %r2
+    %3(s32) = COPY %r3
+    ; HARD-DAG: [[X:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[X0]]
+    ; HARD-DAG: [[Y:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[Y0]]
+    %4(s64) = G_MERGE_VALUES %0(s32), %1(s32)
+    %5(s64) = G_MERGE_VALUES %2(s32), %3(s32)
+    ; HARD: [[R:%[0-9]+]]:_(s64) = G_FSUB [[X]], [[Y]]
+    ; SOFT-NOT: G_FSUB
+    ; SOFT: ADJCALLSTACKDOWN
+    ; SOFT-DAG: %r{{[0-1]}} = COPY [[X0]]
+    ; SOFT-DAG: %r{{[0-1]}} = COPY [[X1]]
+    ; SOFT-DAG: %r{{[2-3]}} = COPY [[Y0]]
+    ; SOFT-DAG: %r{{[2-3]}} = COPY [[Y1]]
+    ; SOFT-AEABI: BL $__aeabi_dsub, {{.*}}, implicit %r0, implicit %r1, implicit %r2, implicit %r3, implicit-def %r0, implicit-def %r1
+    ; SOFT-DEFAULT: BL $__subdf3, {{.*}}, implicit %r0, implicit %r1, implicit %r2, implicit %r3, implicit-def %r0, implicit-def %r1
+    ; SOFT: ADJCALLSTACKUP
+    ; SOFT-NOT: G_FSUB
+    %6(s64) = G_FSUB %4, %5
+    ; HARD-DAG: G_UNMERGE_VALUES [[R]](s64)
+    %7(s32),%8(s32) = G_UNMERGE_VALUES %6(s64)
+    %r0 = COPY %7(s32)
+    %r1 = COPY %8(s32)
+    BX_RET 14, _, implicit %r0, implicit %r1
+...
+---
 name:            test_fcmp_true_s32
 # CHECK-LABEL: name: test_fcmp_true_s32
 legalized:       false
 # CHECK: legalized: true
 regBankSelected: false
 selected:        false
 tracksRegLiveness: true
 registers:

test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir

--- |
 
   define void @test_select_s32() { ret void }
 
   define void @test_br() { ret void }
 
   define void @test_fadd_s32() #0 { ret void }
   define void @test_fadd_s64() #0 { ret void }
 
+  define void @test_fsub_s32() #0 { ret void }
+  define void @test_fsub_s64() #0 { ret void }
+
   define void @test_soft_fp_s64() #0 { ret void }
 
   attributes #0 = { "target-features"="+vfp2"}
   attributes #1 = { "target-features"="+hwdiv-arm" }
 ...
 ---
 name:            test_add_s32
 # CHECK-LABEL: name: test_add_s32
bb.0:
     %0(s64) = COPY %d0
     %1(s64) = COPY %d1
     %2(s64) = G_FADD %0, %1
     %d0 = COPY %2(s64)
     BX_RET 14, _, implicit %d0
 
 ...
 ---
+name:            test_fsub_s32
+# CHECK-LABEL: name: test_fsub_s32
+legalized:       true
+regBankSelected: false
+selected:        false
+# CHECK: registers:
+# CHECK: - { id: 0, class: fprb, preferred-register: '' }
+# CHECK: - { id: 1, class: fprb, preferred-register: '' }
+# CHECK: - { id: 2, class: fprb, preferred-register: '' }
+
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %s0, %s1
+
+    %0(s32) = COPY %s0
+    %1(s32) = COPY %s1
+    %2(s32) = G_FSUB %0, %1
+    %s0 = COPY %2(s32)
+    BX_RET 14, _, implicit %s0
+
+...
+---
+name:            test_fsub_s64
+# CHECK-LABEL: name: test_fsub_s64
+legalized:       true
+regBankSelected: false
+selected:        false
+# CHECK: registers:
+# CHECK: - { id: 0, class: fprb, preferred-register: '' }
+# CHECK: - { id: 1, class: fprb, preferred-register: '' }
+# CHECK: - { id: 2, class: fprb, preferred-register: '' }
+
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %d0, %d1
+
+    %0(s64) = COPY %d0
+    %1(s64) = COPY %d1
+    %2(s64) = G_FSUB %0, %1
+    %d0 = COPY %2(s64)
+    BX_RET 14, _, implicit %d0
+
+...
+---
 name:            test_soft_fp_s64
 # CHECK-LABEL: name: test_soft_fp_s64
 legalized:       true
 regBankSelected: false
 selected:        false
 # CHECK: registers:
 # CHECK: - { id: 0, class: gprb, preferred-register: '' }
 # CHECK: - { id: 1, class: gprb, preferred-register: '' }