[SystemZ] Fix sign of integer memcmp result

r188163 used CLC to implement memcmp.  Code that compares the result
directly against zero can test the CC value produced by CLC, but code
that needs an integer result must use IPM.  The sequence I'd used was:

   ipm <reg>
   sll <reg>, 2
   sra <reg>, 30

but I'd forgotten that this inverts the order, so that CC==1 ("less")
becomes an integer greater than zero, and CC==2 ("greater") becomes
an integer less than zero.  This sequence should only be used if the
CLC arguments are reversed to compensate.  The problem then is that
the branch condition must also be reversed when testing the CLC
result directly.

Rather than do that, I went for a different sequence that works with
the natural CLC order:

   ipm <reg>
   srl <reg>, 28
   rll <reg>, <reg>, 31

One advantage of this is that it doesn't clobber CC.  A disadvantage
is that any sign extension to 64 bits must be done separately,
rather than being folded into the shifts.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@188538 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Target/SystemZ/SystemZInstrInfo.cpp b/lib/Target/SystemZ/SystemZInstrInfo.cpp
index 54a8669359907e140b487c60534d00899c936974..f2e877fd29b01e9eeda702f9de5838b46aa4f5a9 100644 (file)
--- a/lib/Target/SystemZ/SystemZInstrInfo.cpp
+++ b/lib/Target/SystemZ/SystemZInstrInfo.cpp
@@ -311,18 +311,11 @@ bool SystemZInstrInfo::analyzeCompare(const MachineInstr *MI,
   return false;
 }
 
-// If Reg is a virtual register that is used by only a single non-debug
-// instruction, return the defining instruction, otherwise return null.
-static MachineInstr *getDefSingleUse(const MachineRegisterInfo *MRI,
-                                     unsigned Reg) {
+// If Reg is a virtual register, return its definition, otherwise return null.
+static MachineInstr *getDef(unsigned Reg,
+                            const MachineRegisterInfo *MRI) {
   if (TargetRegisterInfo::isPhysicalRegister(Reg))
     return 0;
-
-  MachineRegisterInfo::use_nodbg_iterator I = MRI->use_nodbg_begin(Reg);
-  MachineRegisterInfo::use_nodbg_iterator E = MRI->use_nodbg_end();
-  if (I == E || llvm::next(I) != E)
-    return 0;
-
   return MRI->getUniqueVRegDef(Reg);
 }
 
@@ -333,42 +326,46 @@ static bool isShift(MachineInstr *MI, int Opcode, int64_t Imm) {
           MI->getOperand(3).getImm() == Imm);
 }
 
+// If the destination of MI has no uses, delete it as dead.
+static void eraseIfDead(MachineInstr *MI, const MachineRegisterInfo *MRI) {
+  if (MRI->use_nodbg_empty(MI->getOperand(0).getReg()))
+    MI->eraseFromParent();
+}
+
 // Compare compares SrcReg against zero.  Check whether SrcReg contains
-// the result of an IPM sequence that is only used by Compare.  Try to
-// delete both of them if so and return true if a change was made.
-static bool removeIPM(MachineInstr *Compare, unsigned SrcReg,
-                      const MachineRegisterInfo *MRI,
-                      const TargetRegisterInfo *TRI) {
-  MachineInstr *SRA = getDefSingleUse(MRI, SrcReg);
-  if (!SRA || !isShift(SRA, SystemZ::SRA, 30))
+// the result of an IPM sequence whose input CC survives until Compare,
+// and whether Compare is therefore redundant.  Delete it and return
+// true if so.
+static bool removeIPMBasedCompare(MachineInstr *Compare, unsigned SrcReg,
+                                  const MachineRegisterInfo *MRI,
+                                  const TargetRegisterInfo *TRI) {
+  MachineInstr *RLL = getDef(SrcReg, MRI);
+  if (!RLL || !isShift(RLL, SystemZ::RLL, 31))
     return false;
 
-  MachineInstr *SLL = getDefSingleUse(MRI, SRA->getOperand(1).getReg());
-  if (!SLL || !isShift(SLL, SystemZ::SLL, 2))
+  MachineInstr *SRL = getDef(RLL->getOperand(1).getReg(), MRI);
+  if (!SRL || !isShift(SRL, SystemZ::SRL, 28))
     return false;
 
-  MachineInstr *IPM = getDefSingleUse(MRI, SLL->getOperand(1).getReg());
+  MachineInstr *IPM = getDef(SRL->getOperand(1).getReg(), MRI);
   if (!IPM || IPM->getOpcode() != SystemZ::IPM)
     return false;
 
   // Check that there are no assignments to CC between the IPM and Compare,
-  // except for the SRA that we'd like to delete.  We can ignore SLL because
-  // it does not assign to CC.  We can also ignore uses of the SRA CC result,
-  // since it is effectively restoring CC to the value it had before IPM
-  // (for all current use cases).
   if (IPM->getParent() != Compare->getParent())
     return false;
   MachineBasicBlock::iterator MBBI = IPM, MBBE = Compare;
   for (++MBBI; MBBI != MBBE; ++MBBI) {
     MachineInstr *MI = MBBI;
-    if (MI != SRA && MI->modifiesRegister(SystemZ::CC, TRI))
+    if (MI->modifiesRegister(SystemZ::CC, TRI))
       return false;
   }
 
-  IPM->eraseFromParent();
-  SLL->eraseFromParent();
-  SRA->eraseFromParent();
   Compare->eraseFromParent();
+  eraseIfDead(RLL, MRI);
+  eraseIfDead(SRL, MRI);
+  eraseIfDead(IPM, MRI);
+
   return true;
 }
 
@@ -381,7 +378,7 @@ SystemZInstrInfo::optimizeCompareInstr(MachineInstr *Compare,
   bool IsLogical = (Compare->getDesc().TSFlags & SystemZII::IsLogical) != 0;
   if (Value == 0 &&
       !IsLogical &&
-      removeIPM(Compare, SrcReg, MRI, TM.getRegisterInfo()))
+      removeIPMBasedCompare(Compare, SrcReg, MRI, TM.getRegisterInfo()))
     return true;
   return false;
 }

diff --git a/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp b/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp
index 341dc9465502a0e2c14bc556a9b6c634235724f3..825153caa7fd58976103611f9082ef6da989ffdb 100644 (file)
--- a/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp
+++ b/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp
@@ -140,14 +140,15 @@ EmitTargetCodeForMemcmp(SelectionDAG &DAG, SDLoc DL, SDValue Chain,
                           Src1, Src2, Size);
       SDValue Glue = Chain.getValue(1);
       // IPM inserts the CC value into bits 29 and 28, with 0 meaning "equal",
-      // 1 meaning "greater" and 2 meaning "less".  Convert them into an
-      // integer that is respectively equal, greater or less than 0.
+      // 1 meaning "less" and 2 meaning "greater".  Bits 30 and 31 are zero.
+      // Convert this into an integer that is respectively equal, less
+      // or greater than 0.
       SDValue IPM = DAG.getNode(SystemZISD::IPM, DL, MVT::i32, Glue);
-      SDValue SHL = DAG.getNode(ISD::SHL, DL, MVT::i32, IPM,
-                                DAG.getConstant(2, MVT::i32));
-      SDValue SRA = DAG.getNode(ISD::SRA, DL, MVT::i32, SHL,
-                                DAG.getConstant(30, MVT::i32));
-      return std::make_pair(SRA, Chain);
+      SDValue SRL = DAG.getNode(ISD::SRL, DL, MVT::i32, IPM,
+                                DAG.getConstant(28, MVT::i32));
+      SDValue ROTL = DAG.getNode(ISD::ROTL, DL, MVT::i32, SRL,
+                                 DAG.getConstant(31, MVT::i32));
+      return std::make_pair(ROTL, Chain);
     }
   }
   return std::make_pair(SDValue(), SDValue());

diff --git a/test/CodeGen/SystemZ/memcmp-01.ll b/test/CodeGen/SystemZ/memcmp-01.ll
index 37477695b78a10f7dbf5c00336c7c2468a5a09bc..4d8cd14880a77d620115feea9b66929b18985758 100644 (file)
--- a/test/CodeGen/SystemZ/memcmp-01.ll
+++ b/test/CodeGen/SystemZ/memcmp-01.ll
@@ -17,9 +17,9 @@ define i32 @f1(i8 *%src1, i8 *%src2) {
 define i32 @f2(i8 *%src1, i8 *%src2) {
 ; CHECK-LABEL: f2:
 ; CHECK: clc 0(2,%r2), 0(%r3)
-; CHECK: ipm %r2
-; CHECK: sll %r2, 2
-; CHECK: sra %r2, 30
+; CHECK: ipm [[REG:%r[0-5]]]
+; CHECK: srl [[REG]], 28
+; CHECK: rll %r2, [[REG]], 31
 ; CHECK: br %r14
   %res = call i32 @memcmp(i8 *%src1, i8 *%src2, i64 2)
   ret i32 %res
@@ -101,14 +101,13 @@ exit:
 }
 
 ; Check the upper end of the CLC range.  Here the result is used both as
-; an integer and for branching, but it's better to branch on the result
-; of the SRA.
+; an integer and for branching.
 define i32 @f7(i8 *%src1, i8 *%src2, i32 *%dest) {
 ; CHECK-LABEL: f7:
 ; CHECK: clc 0(256,%r2), 0(%r3)
-; CHECK: ipm %r2
-; CHECK: sll %r2, 2
-; CHECK: sra %r2, 30
+; CHECK: ipm [[REG:%r[0-5]]]
+; CHECK: srl [[REG]], 28
+; CHECK: rll %r2, [[REG]], 31
 ; CHECK: jl {{.L*}}
 ; CHECK: br %r14
 entry: