[SystemZ] Use VGM whenever possible to load FP immediates

[jonpa]

Summary

VGM (Vector Generate Mask) can be used to build certain FP immediates.

isFPImmLegal() extended to recognize these values and return true.

New pseudo instructions with a custom inserter use VGM and then copy the right subreg to the FP virtual register.

Tests updated + new test: fp-const-12.ll.

[jonpa]

Patch was proven to be incorrect by an assertion failure.

Updated to take better care with bitwidths (double / float) by using VGMG / VGMF.

[uweigand]

It seems you're assuming VGM is always available. I guess there needs to be a check for hasVector() somewhere.

Is there a reason why you're only creating the VGM at the MI level via a custom inserter, instead of introducing it during late DAG matching like we do for vector types? It seems odd to handle the scalar and vector cases differently here ...

In general, this does look like a good idea.

[jonpa]

It seems you're assuming VGM is always available. I guess there needs to be a check for hasVector() somewhere.

Yes, fixed.

Is there a reason why you're only creating the VGM at the MI level via a custom inserter, instead of introducing it during late DAG matching like we do for vector types? It seems odd to handle the scalar and vector cases differently here ...

At first that seemed simpler to me, but I agree now that it is better to do it in Select(). Interestingly enough, this change gives a slightly different code generation:

                             CustomInserter vs Select()
mdbr           :                 7040                 6957      -83
wfmdb          :                19484                19561      +77
wfadb          :                10173                10240      +67
vsteg          :                 1774                 1720      -54
adbr           :                10648                10595      -53
vlrepg         :                 4836                 4795      -41
std            :                23026                23057      +31
ld             :                34156                34176      +20
vl             :                23472                23490      +18
vgmg           :                 3999                 3982      -17
lr             :                29745                29729      -16
cdbr           :                 7785                 7800      +15
lgr            :               355323               355309      -14
lg             :               374064               374052      -12
jne            :                14047                14059      +12
vgmf           :                  866                  860       -6
sdbr           :                 3122                 3127       +5
wldeb          :                  326                  321       -5
ltg            :                49842                49847       +5
...
Spill|Reload   :               189197               189161      -36

After isel, we now do the VGM with a vr128bit register, whereas the custom inserter used a vf128 register.

I was able to make a reduced test case from a file where the number of VGM?s was different now. In this one case trunk (and previous version of patch) had a loop invariant instruction hoisted by MachineLICM, whereas now it did not. It was due to some reg pressure heuristic. Not sure if this is important overall... Using vr128 bit seems right, by thereby giving regalloc more freedom...

[uweigand]

Yes, I think this version is better.

LGTM, thanks!

[jonpa]

Thanks for review. r353867.

lib/Target/SystemZ/SystemZISelLowering.h

private:
   MachineBasicBlock *emitStringWrapper(MachineInstr &MI, MachineBasicBlock *BB,
                                        unsigned Opcode) const;
   MachineBasicBlock *emitTransactionBegin(MachineInstr &MI,
                                           MachineBasicBlock *MBB,
                                           unsigned Opcode, bool NoFloat) const;
   MachineBasicBlock *emitLoadAndTestCmp0(MachineInstr &MI,
                                          MachineBasicBlock *MBB,
                                          unsigned Opcode) const;
+  MachineBasicBlock *emitFPScalarImm(MachineInstr &MI,
+                                     MachineBasicBlock *MBB) const;
 
   const TargetRegisterClass *getRepRegClassFor(MVT VT) const override;
 };
 } // end namespace llvm
 
 #endif

lib/Target/SystemZ/SystemZISelLowering.cpp

case MVT::f128:
     return Subtarget.hasVectorEnhancements1();
   default:
     break;
   }
 
   return false;
 }
 
+static bool analyzeFPImm(const APFloat &Imm, unsigned &Start, unsigned &End,
+                         const SystemZInstrInfo *TII) {
+  APInt IntImm = Imm.bitcastToAPInt();
+  if (IntImm.getActiveBits() > 64)
+    return false;
+
+  // See if this immediate could be generated with VGM.
+  return TII->isRxSBGMask(uint64_t(IntImm.getZExtValue()), 64, Start, End);
+}
+
 bool SystemZTargetLowering::isFPImmLegal(const APFloat &Imm, EVT VT) const {
   // We can load zero using LZ?R and negative zero using LZ?R;LC?BR.
-  return Imm.isZero() || Imm.isNegZero();
+  if (Imm.isZero() || Imm.isNegZero())
+    return true;
+
+  const SystemZInstrInfo *TII =
+      static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
+  unsigned Start, End;
+  return analyzeFPImm(Imm, Start, End, TII);
 }
 
 bool SystemZTargetLowering::isLegalICmpImmediate(int64_t Imm) const {
   // We can use CGFI or CLGFI.
   return isInt<32>(Imm) || isUInt<32>(Imm);
 }
 
 bool SystemZTargetLowering::isLegalAddImmediate(int64_t Imm) const {
MachineBasicBlock *SystemZTargetLowering::emitLoadAndTestCmp0(
   // well.
   BuildMI(*MBB, MI, DL, TII->get(Opcode), DstReg)
     .addReg(SrcReg);
   MI.eraseFromParent();
 
   return MBB;
 }
 
+MachineBasicBlock *SystemZTargetLowering::emitFPScalarImm(
+    MachineInstr &MI, MachineBasicBlock *MBB) const {
+  MachineFunction &MF = *MBB->getParent();
+  MachineRegisterInfo *MRI = &MF.getRegInfo();
+  const SystemZInstrInfo *TII =
+      static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
+
+  APFloat Imm = MI.getOperand(1).getFPImm()->getValueAPF();
+  assert(!Imm.isZero() && !Imm.isNegZero() && "Expected non-zero FP immediate");
+  unsigned Start, End;
+  bool Success = analyzeFPImm(Imm, Start, End, TII);
+  assert(Success && "Can't build FP immediate.");
+
+  unsigned DstReg = MI.getOperand(0).getReg();
+  unsigned SubRegIdx = (MRI->getRegClass(DstReg) == &SystemZ::FP32BitRegClass ?
+                        SystemZ::subreg_h32 : SystemZ::subreg_h64);
+  unsigned VReg = MRI->createVirtualRegister(&SystemZ::VF128BitRegClass);
+  DebugLoc DL = MI.getDebugLoc();
+  BuildMI(*MBB, MI, DL, TII->get(SystemZ::VGMG), VReg)
+    .addImm(Start)
+    .addImm(End);
+  BuildMI(*MBB, MI, DL, TII->get(TargetOpcode::COPY), DstReg)
+    .addReg(VReg, RegState::Kill, SubRegIdx);
+
+  MI.eraseFromParent();
+  return MBB;
+}
+
 MachineBasicBlock *SystemZTargetLowering::EmitInstrWithCustomInserter(
     MachineInstr &MI, MachineBasicBlock *MBB) const {
   switch (MI.getOpcode()) {
   case SystemZ::Select32:
   case SystemZ::Select64:
   case SystemZ::SelectF32:
   case SystemZ::SelectF64:
   case SystemZ::SelectF128:
case SystemZ::LTDBRCompare_VecPseudo:
     return emitLoadAndTestCmp0(MI, MBB, SystemZ::LTDBR);
   case SystemZ::LTXBRCompare_VecPseudo:
     return emitLoadAndTestCmp0(MI, MBB, SystemZ::LTXBR);
 
   case TargetOpcode::STACKMAP:
   case TargetOpcode::PATCHPOINT:
     return emitPatchPoint(MI, MBB);
 
+  case SystemZ::FP32ScalarImmPseudo:
+  case SystemZ::FP64ScalarImmPseudo:
+    return emitFPScalarImm(MI, MBB);
+
   default:
     llvm_unreachable("Unexpected instr type to insert");
   }
 }
 
 // This is only used by the isel schedulers, and is needed only to prevent
 // compiler from crashing when list-ilp is used.
 const TargetRegisterClass *
 SystemZTargetLowering::getRepRegClassFor(MVT VT) const {
   if (VT == MVT::Untyped)
     return &SystemZ::ADDR128BitRegClass;
   return TargetLowering::getRepRegClassFor(VT);
 }

lib/Target/SystemZ/SystemZInstrFP.td

 
 // Load zero.
 let isAsCheapAsAMove = 1, isMoveImm = 1 in {
   def LZER : InherentRRE<"lzer", 0xB374, FP32,  fpimm0>;
   def LZDR : InherentRRE<"lzdr", 0xB375, FP64,  fpimm0>;
   def LZXR : InherentRRE<"lzxr", 0xB376, FP128, fpimm0>;
 }
 
+// Load scalar floating-point immediate with a VGM.
+let isAsCheapAsAMove = 1, isMoveImm = 1, usesCustomInserter = 1,
+    hasNoSchedulingInfo = 1 in {
+  def FP32ScalarImmPseudo : Pseudo<(outs FP32:$R1), (ins FP32:$Imm),
+                                   [(set FP32:$R1, (fpimm:$Imm))]>;
+  def FP64ScalarImmPseudo : Pseudo<(outs FP64:$R1), (ins FP64:$Imm),
+                                   [(set FP64:$R1, (fpimm:$Imm))]>;
+}
+
 // Moves between two floating-point registers.
 def LER : UnaryRR <"ler", 0x38,   null_frag, FP32,  FP32>;
 def LDR : UnaryRR <"ldr", 0x28,   null_frag, FP64,  FP64>;
 def LXR : UnaryRRE<"lxr", 0xB365, null_frag, FP128, FP128>;
 
 // For z13 we prefer LDR over LER to avoid partial register dependencies.
 let isCodeGenOnly = 1 in
   def LDR32 : UnaryRR<"ldr", 0x28, null_frag, FP32, FP32>;

test/CodeGen/SystemZ/args-07.ll

 ; CHECK: br %r14
   ret { i64, i64, i64, i64, i64 } { i64 0, i64 1, i64 2, i64 3, i64 4 }
 }
 
 ; Up to four floating-point return values fit into FPRs.
 define { double, double, double, double } @f3() {
 ; CHECK-LABEL: f3:
 ; CHECK: larl [[TMP:%r[0-5]]], .LCPI
-; CHECK: ldeb %f0, 0([[TMP]])
-; CHECK: larl [[TMP:%r[0-5]]], .LCPI
-; CHECK: ldeb %f2, 0([[TMP]])
-; CHECK: larl [[TMP:%r[0-5]]], .LCPI
 ; CHECK: ldeb %f4, 0([[TMP]])
 ; CHECK: larl [[TMP:%r[0-5]]], .LCPI
 ; CHECK: ldeb %f6, 0([[TMP]])
+; CHECK: vgmg %v0, 2, 11
+; CHECK: vgmg %v2, 1, 1
 ; CHECK: br %r14
   ret { double, double, double, double }
       { double 1.0, double 2.0, double 3.0, double 4.0 }
 }
 
 ; More than four floating-point return values use sret.
 define { double, double, double, double, double } @f4() {
 ; CHECK-LABEL: f4:

test/CodeGen/SystemZ/fp-const-12.ll

+; Test loads of FP constants with VGM.
+;
+; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z13 | FileCheck %s
+
+define double @f1() {
+; CHECK-LABEL: f1:
+; CHECK: vgmg %v0, 2, 11
+  ret double 1.0
+}
+
+define double @f2() {
+; CHECK-LABEL: f2:
+; CHECK: vgmg %v0, 1, 1
+  ret double 2.0
+}
+
+define double @f3() {
+; CHECK-LABEL: f3:
+; CHECK: vgmg %v0, 0, 1
+  ret double -2.0
+}
+
+define double @f4() {
+; CHECK-LABEL: f4:
+; CHECK: vgmg %v0, 2, 10
+  ret double 0.5
+}
+
+define double @f5() {
+; CHECK-LABEL: f5:
+; CHECK: vgmg %v0, 2, 9
+  ret double 0.125
+}
+
+define float @f6() {
+; CHECK-LABEL: f6:
+; CHECK: vgmg %v0, 34, 40
+  ret float 1.0
+}
+
+define float @f7() {
+; CHECK-LABEL: f7:
+; CHECK: vgmg %v0, 33, 33
+  ret float 2.0
+}
+
+define float @f8() {
+; CHECK-LABEL: f8:
+; CHECK: vgmg %v0, 32, 33
+  ret float -2.0
+}
+
+define float @f9() {
+; CHECK-LABEL: f9:
+; CHECK: vgmg %v0, 34, 39
+  ret float 0.5
+}
+
+define float @f10() {
+; CHECK-LABEL: f10:
+; CHECK: vgmg %v0, 34, 38
+  ret float 0.125
+}

test/CodeGen/SystemZ/subregliveness-02.ll

 ; RUN: llc -mtriple=s390x-linux-gnu -mcpu=z13 -systemz-subreg-liveness < %s | FileCheck %s
 
 ; Check for successful compilation.
-; CHECK: meeb %f0, 0(%r1)
+; CHECK: meebr %f1, %f0
 
 target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
 target triple = "s390x-ibm-linux"
 
 ; Function Attrs: nounwind
 define void @spec_random_load() #0 {
 bb:
   %tmp = sitofp i64 undef to float

test/CodeGen/SystemZ/swifterror.ll

 ; "caller2" is the caller of "foo", it calls "foo" inside a loop.
 define float @caller2(i8* %error_ref) {
 ; CHECK-LABEL: caller2:
 ; Make a copy of error_ref because r2 is getting clobbered
 ; CHECK: lgr %r[[REG1:[0-9]+]], %r2
 ; CHECK: lghi %r9, 0
 ; CHECK: brasl %r14, foo
 ; CHECK: cgijlh %r9, 0,
-; CHECK: ceb %f0,
+; CHECK: cebr %f0,
 ; CHECK: jnh
 ; Access part of the error object and save it to error_ref
 ; CHECK: lb %r[[REG2:[0-9]+]], 8(%r9)
 ; CHECK: stc %r[[REG2]], 0(%r[[REG1]])
 ; CHECK: lgr %r2, %r9
 ; CHECK: brasl %r14, free
 ; CHECK-O0-LABEL: caller2:
 ; CHECK-O0: lghi %r9, 0

 ; under a certain condition inside a loop.
 define float @foo_loop(%swift_error** swifterror %error_ptr_ref, i32 %cc, float %cc2) {
 ; CHECK-LABEL: foo_loop:
 ; CHECK: lr %r[[REG1:[0-9]+]], %r2
 ; CHECK: cije %r[[REG1]], 0
 ; CHECK: lghi %r2, 16
 ; CHECK: brasl %r14, malloc
 ; CHECK: mvi 8(%r2), 1
-; CHECK: ceb %f8,
+; CHECK: cebr %f8,
 ; CHECK: jnh
 ; CHECK: lgr %r9, %r2
 ; CHECK: br %r14
 ; CHECK-O0-LABEL: foo_loop:
 ; spill to stack
 ; CHECK-O0: stg %r9, [[OFFS:[0-9]+]](%r15)
 ; CHECK-O0: chi %r{{.*}}, 0
 ; CHECK-O0: je

test/CodeGen/SystemZ/tdc-03.ll

; CHECK-NOT: tceb
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare with inf (unworthy)
 define i32 @f3(float %x) {
 ; CHECK-LABEL: f3
 ; CHECK-NOT: tceb
-; CHECK: ceb %f0, 0(%r{{[0-9]+}})
+; CHECK: cebr %f0, %f{{[0-9]+}}
 ; CHECK-NOT: tceb
   %res = fcmp ult float %x, 0x7ff0000000000000
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare fabs with inf
 define i32 @f4(float %x) {
 ; CHECK-LABEL: f4
 ; CHECK: tceb %f0, 4047
   %y = call float @llvm.fabs.f32(float %x)
   %res = fcmp ult float %y, 0x7ff0000000000000
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare with minnorm (unworthy)
 define i32 @f5(float %x) {
 ; CHECK-LABEL: f5
 ; CHECK-NOT: tceb
-; CHECK: ceb %f0, 0(%r{{[0-9]+}})
+; CHECK: cebr %f0, %f{{[0-9]+}}
 ; CHECK-NOT: tceb
   %res = fcmp ult float %x, 0x3810000000000000
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare fabs with minnorm
 define i32 @f6(float %x) {
 ; CHECK-LABEL: f6
 ; CHECK: tceb %f0, 3279
   %y = call float @llvm.fabs.f32(float %x)
   %res = fcmp ult float %y, 0x3810000000000000
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare fabs with minnorm, unsupported condition
 define i32 @f7(float %x) {
 ; CHECK-LABEL: f7
 ; CHECK-NOT: tceb
 ; CHECK: lpdfr [[REG:%f[0-9]+]], %f0
-; CHECK: ceb [[REG]], 0(%r{{[0-9]+}})
+; CHECK: cebr [[REG]], %f{{[0-9]+}}
 ; CHECK-NOT: tceb
   %y = call float @llvm.fabs.f32(float %x)
   %res = fcmp ugt float %y, 0x3810000000000000
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare fabs with unsupported constant
 define i32 @f8(float %x) {
 ; CHECK-LABEL: f8
 ; CHECK-NOT: tceb
 ; CHECK: lpdfr [[REG:%f[0-9]+]], %f0
-; CHECK: ceb [[REG]], 0(%r{{[0-9]+}})
+; CHECK: cebr [[REG]], %f{{[0-9]+}}
 ; CHECK-NOT: tceb
   %y = call float @llvm.fabs.f32(float %x)
   %res = fcmp ult float %y, 0x3ff0000000000000
   %xres = zext i1 %res to i32
   ret i32 %xres
 }
 
 ; Compare fabs with minnorm - double