Revert "X86InstrInfo: Support immediates that are +1/-1 different in …

…optimizeCompareInstr"

This reverts commit 847a680.

The reverted revision was causing miscompiles that manifest on AMD
machines.

Differential Revision: https://reviews.llvm.org/D115528

llvm/lib/Target/X86/X86InstrInfo.cpp

@@ -4088,8 +4088,8 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, Register &SrcReg,
 bool X86InstrInfo::isRedundantFlagInstr(const MachineInstr &FlagI,
                                         Register SrcReg, Register SrcReg2,
                                         int64_t ImmMask, int64_t ImmValue,
-                                        const MachineInstr &OI, bool *IsSwapped,
-                                        int64_t *ImmDelta) const {
+                                        const MachineInstr &OI,
+                                        bool *IsSwapped) const {
   switch (OI.getOpcode()) {
   case X86::CMP64rr:
   case X86::CMP32rr:
@@ -4140,21 +4140,10 @@ bool X86InstrInfo::isRedundantFlagInstr(const MachineInstr &FlagI,
       int64_t OIMask;
       int64_t OIValue;
       if (analyzeCompare(OI, OISrcReg, OISrcReg2, OIMask, OIValue) &&
-          SrcReg == OISrcReg && ImmMask == OIMask) {
-        if (OIValue == ImmValue) {
-          *ImmDelta = 0;
-          return true;
-        } else if (static_cast<uint64_t>(ImmValue) ==
-                   static_cast<uint64_t>(OIValue) - 1) {
-          *ImmDelta = -1;
-          return true;
-        } else if (static_cast<uint64_t>(ImmValue) ==
-                   static_cast<uint64_t>(OIValue) + 1) {
-          *ImmDelta = 1;
-          return true;
-        } else {
-          return false;
-        }
+          SrcReg == OISrcReg && ImmMask == OIMask && OIValue == ImmValue) {
+        assert(SrcReg2 == X86::NoRegister && OISrcReg2 == X86::NoRegister &&
+               "should not have 2nd register");
+        return true;
       }
     }
     return FlagI.isIdenticalTo(OI);
@@ -4404,7 +4393,6 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
   bool ShouldUpdateCC = false;
   bool IsSwapped = false;
   X86::CondCode NewCC = X86::COND_INVALID;
-  int64_t ImmDelta = 0;
 
   // Search backward from CmpInstr for the next instruction defining EFLAGS.
   const TargetRegisterInfo *TRI = &getRegisterInfo();
@@ -4451,7 +4439,7 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
         //     ...           // EFLAGS not changed
         //     cmp x, y      // <-- can be removed
         if (isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpMask, CmpValue,
-                                 Inst, &IsSwapped, &ImmDelta)) {
+                                 Inst, &IsSwapped)) {
           Sub = &Inst;
           break;
         }
@@ -4485,7 +4473,7 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
   // It is safe to remove CmpInstr if EFLAGS is redefined or killed.
   // If we are done with the basic block, we need to check whether EFLAGS is
   // live-out.
-  bool FlagsMayLiveOut = true;
+  bool IsSafe = false;
   SmallVector<std::pair<MachineInstr*, X86::CondCode>, 4> OpsToUpdate;
   MachineBasicBlock::iterator AfterCmpInstr =
       std::next(MachineBasicBlock::iterator(CmpInstr));
@@ -4495,15 +4483,15 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
     // We should check the usage if this instruction uses and updates EFLAGS.
     if (!UseEFLAGS && ModifyEFLAGS) {
       // It is safe to remove CmpInstr if EFLAGS is updated again.
-      FlagsMayLiveOut = false;
+      IsSafe = true;
       break;
     }
     if (!UseEFLAGS && !ModifyEFLAGS)
       continue;
 
     // EFLAGS is used by this instruction.
     X86::CondCode OldCC = X86::COND_INVALID;
-    if (MI || IsSwapped || ImmDelta != 0) {
+    if (MI || IsSwapped) {
       // We decode the condition code from opcode.
       if (Instr.isBranch())
         OldCC = X86::getCondFromBranch(Instr);
@@ -4555,75 +4543,26 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
       // to be changed from r2 > r1 to r1 < r2, from r2 < r1 to r1 > r2, etc.
       // We swap the condition code and synthesize the new opcode.
       ReplacementCC = getSwappedCondition(OldCC);
-      if (ReplacementCC == X86::COND_INVALID) return false;
-      ShouldUpdateCC = true;
-    } else if (ImmDelta != 0) {
-      unsigned BitWidth = TRI->getRegSizeInBits(*MRI->getRegClass(SrcReg));
-      // Shift amount for min/max constants to adjust for 8/16/32 instruction
-      // sizes.
-      switch (OldCC) {
-      case X86::COND_L: // x <s (C + 1)  -->  x <=s C
-        if (ImmDelta != 1 || APInt::getSignedMinValue(BitWidth) == CmpValue)
-          return false;
-        ReplacementCC = X86::COND_LE;
-        break;
-      case X86::COND_B: // x <u (C + 1)  -->  x <=u C
-        if (ImmDelta != 1 || CmpValue == 0)
-          return false;
-        ReplacementCC = X86::COND_BE;
-        break;
-      case X86::COND_GE: // x >=s (C + 1)  -->  x >s C
-        if (ImmDelta != 1 || APInt::getSignedMinValue(BitWidth) == CmpValue)
-          return false;
-        ReplacementCC = X86::COND_G;
-        break;
-      case X86::COND_AE: // x >=u (C + 1)  -->  x >u C
-        if (ImmDelta != 1 || CmpValue == 0)
-          return false;
-        ReplacementCC = X86::COND_A;
-        break;
-      case X86::COND_G: // x >s (C - 1)  -->  x >=s C
-        if (ImmDelta != -1 || APInt::getSignedMaxValue(BitWidth) == CmpValue)
-          return false;
-        ReplacementCC = X86::COND_GE;
-        break;
-      case X86::COND_A: // x >u (C - 1)  -->  x >=u C
-        if (ImmDelta != -1 || APInt::getMaxValue(BitWidth) == CmpValue)
-          return false;
-        ReplacementCC = X86::COND_AE;
-        break;
-      case X86::COND_LE: // x <=s (C - 1)  -->  x <s C
-        if (ImmDelta != -1 || APInt::getSignedMaxValue(BitWidth) == CmpValue)
-          return false;
-        ReplacementCC = X86::COND_L;
-        break;
-      case X86::COND_BE: // x <=u (C - 1)  -->  x <u C
-        if (ImmDelta != -1 || APInt::getMaxValue(BitWidth) == CmpValue)
-          return false;
-        ReplacementCC = X86::COND_B;
-        break;
-      default:
+      if (ReplacementCC == X86::COND_INVALID)
         return false;
-      }
-      ShouldUpdateCC = true;
     }
 
-    if (ShouldUpdateCC && ReplacementCC != OldCC) {
+    if ((ShouldUpdateCC || IsSwapped) && ReplacementCC != OldCC) {
       // Push the MachineInstr to OpsToUpdate.
       // If it is safe to remove CmpInstr, the condition code of these
       // instructions will be modified.
       OpsToUpdate.push_back(std::make_pair(&Instr, ReplacementCC));
     }
     if (ModifyEFLAGS || Instr.killsRegister(X86::EFLAGS, TRI)) {
       // It is safe to remove CmpInstr if EFLAGS is updated again or killed.
-      FlagsMayLiveOut = false;
+      IsSafe = true;
       break;
     }
   }
 
-  // If we have to update users but EFLAGS is live-out abort, since we cannot
-  // easily find all of the users.
-  if (ShouldUpdateCC && FlagsMayLiveOut) {
+  // If EFLAGS is not killed nor re-defined, we should check whether it is
+  // live-out. If it is live-out, do not optimize.
+  if ((MI || IsSwapped) && !IsSafe) {
     for (MachineBasicBlock *Successor : CmpMBB.successors())
       if (Successor->isLiveIn(X86::EFLAGS))
         return false;

llvm/lib/Target/X86/X86InstrInfo.h

@@ -643,8 +643,7 @@ class X86InstrInfo final : public X86GenInstrInfo {
   ///   CMP %1, %2   and  %3 = SUB %2, %1  ; IsSwapped=true
   bool isRedundantFlagInstr(const MachineInstr &FlagI, Register SrcReg,
                             Register SrcReg2, int64_t ImmMask, int64_t ImmValue,
-                            const MachineInstr &OI, bool *IsSwapped,
-                            int64_t *ImmDelta) const;
+                            const MachineInstr &OI, bool *IsSwapped) const;
 };
 
 } // namespace llvm