[llvm] r188538 - [SystemZ] Fix sign of integer memcmp result

Richard Sandiford rsandifo at linux.vnet.ibm.com
Fri Aug 16 03:22:54 PDT 2013


Author: rsandifo
Date: Fri Aug 16 05:22:54 2013
New Revision: 188538

URL: http://llvm.org/viewvc/llvm-project?rev=188538&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.cpp
    llvm/trunk/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp
    llvm/trunk/test/CodeGen/SystemZ/memcmp-01.ll

Modified: llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.cpp?rev=188538&r1=188537&r2=188538&view=diff
==============================================================================
--- llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.cpp Fri Aug 16 05:22:54 2013
@@ -311,18 +311,11 @@ bool SystemZInstrInfo::analyzeCompare(co
   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, in
           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(M
   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;
 }

Modified: llvm/trunk/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp?rev=188538&r1=188537&r2=188538&view=diff
==============================================================================
--- llvm/trunk/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp (original)
+++ llvm/trunk/lib/Target/SystemZ/SystemZSelectionDAGInfo.cpp Fri Aug 16 05:22:54 2013
@@ -140,14 +140,15 @@ EmitTargetCodeForMemcmp(SelectionDAG &DA
                           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());

Modified: llvm/trunk/test/CodeGen/SystemZ/memcmp-01.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/memcmp-01.ll?rev=188538&r1=188537&r2=188538&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/SystemZ/memcmp-01.ll (original)
+++ llvm/trunk/test/CodeGen/SystemZ/memcmp-01.ll Fri Aug 16 05:22:54 2013
@@ -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:





More information about the llvm-commits mailing list