[X86] Use lock add/sub for cases that we only care about the EFLAGS

[pengfei]

Summary

This fixes #36373, #36905 and partial of #58685.

[RKSimon]

llvm/lib/Target/X86/X86ISelLowering.cpp

31340

m_OneUse(m_c_Add(...)) ?

31348

m_OneUse(m_Sub(...))?

31368

assert ICI->getPredicate() for EQ/SLT ?

llvm/test/CodeGen/X86/pr58685.ll

4

Cleanup the method names and drop the dso_local / atttributes etc?

[pengfei]

Address review comments. Thanks @RKSimon !

[pengfei]

Fix typo.

[RKSimon]

LGTM (with the test precommit) - cheers

llvm/test/CodeGen/X86/pr58685.ll

50

Please pre-commit these tests with current codegen and rebase the patch so that the commit shows the codegen diff

llvm/include/llvm/CodeGen/TargetLowering.h

enum class AtomicExpansionKind {
     LLSC,    // Expand the instruction into loadlinked/storeconditional; used
              // by ARM/AArch64.
     LLOnly,  // Expand the (load) instruction into just a load-linked, which has
              // greater atomic guarantees than a normal load.
     CmpXChg, // Expand the instruction into cmpxchg; used by at least X86.
     MaskedIntrinsic,  // Use a target-specific intrinsic for the LL/SC loop.
     BitTestIntrinsic, // Use a target-specific intrinsic for special bit
                       // operations; used by X86.
+    CmpArithIntrinsic,// Use a target-specific intrinsic for special compare
+                      // operations; used by X86.
     Expand,           // Generic expansion in terms of other atomic operations.
 
     // Rewrite to a non-atomic form for use in a known non-preemptible
     // environment.
     NotAtomic
   };
 
   /// Enum that specifies when a multiplication should be expanded.
public:
   /// Perform a bit test atomicrmw using a target-specific intrinsic. This
   /// represents the combined bit test intrinsic which will be lowered at a late
   /// stage by the backend.
   virtual void emitBitTestAtomicRMWIntrinsic(AtomicRMWInst *AI) const {
     llvm_unreachable(
         "Bit test atomicrmw expansion unimplemented on this target");
   }
 
+  /// Perform a atomicrmw which the result is only used by comparison, using a
+  /// target-specific intrinsic. This represents the combined atomic and compare
+  /// intrinsic which will be lowered at a late stage by the backend.
+  virtual void emitCmpArithAtomicRMWIntrinsic(AtomicRMWInst *AI) const {
+    llvm_unreachable(
+        "Compare arith atomicrmw expansion unimplemented on this target");
+  }
+
   /// Perform a masked cmpxchg using a target-specific intrinsic. This
   /// represents the core LL/SC loop which will be lowered at a late stage by
   /// the backend. The target-specific intrinsic returns the loaded value and
   /// is not responsible for masking and shifting the result.
   virtual Value *emitMaskedAtomicCmpXchgIntrinsic(
       IRBuilderBase &Builder, AtomicCmpXchgInst *CI, Value *AlignedAddr,
       Value *CmpVal, Value *NewVal, Value *Mask, AtomicOrdering Ord) const {
     llvm_unreachable("Masked cmpxchg expansion unimplemented on this target");

llvm/include/llvm/IR/IntrinsicsX86.td

let TargetPrefix = "x86" in {
   def int_x86_atomic_bts : Intrinsic<[llvm_anyint_ty], [llvm_ptr_ty, llvm_i8_ty],
                                      [ImmArg<ArgIndex<1>>]>;
   def int_x86_atomic_btc : Intrinsic<[llvm_anyint_ty], [llvm_ptr_ty, llvm_i8_ty],
                                      [ImmArg<ArgIndex<1>>]>;
   def int_x86_atomic_btr : Intrinsic<[llvm_anyint_ty], [llvm_ptr_ty, llvm_i8_ty],
                                      [ImmArg<ArgIndex<1>>]>;
 }
 
+// Lock binary arith with CC.
+let TargetPrefix = "x86" in {
+  def int_x86_atomic_add_cc  : Intrinsic<[llvm_i8_ty], [llvm_ptr_ty, llvm_anyint_ty, llvm_i32_ty],
+                                         [ImmArg<ArgIndex<2>>]>;
+  def int_x86_atomic_sub_cc  : Intrinsic<[llvm_i8_ty], [llvm_ptr_ty, llvm_anyint_ty, llvm_i32_ty],
+                                         [ImmArg<ArgIndex<2>>]>;
+}
+
 // Read Processor Register.
 let TargetPrefix = "x86" in {
   def int_x86_rdpru : ClangBuiltin<"__builtin_ia32_rdpru">,
               Intrinsic<[llvm_i64_ty], [llvm_i32_ty], []>;
 }
 
 //===----------------------------------------------------------------------===//
 // CET SS

llvm/lib/CodeGen/AtomicExpandPass.cpp

bool AtomicExpand::tryExpandAtomicRMW(AtomicRMWInst *AI) {
   case TargetLoweringBase::AtomicExpansionKind::MaskedIntrinsic: {
     expandAtomicRMWToMaskedIntrinsic(AI);
     return true;
   }
   case TargetLoweringBase::AtomicExpansionKind::BitTestIntrinsic: {
     TLI->emitBitTestAtomicRMWIntrinsic(AI);
     return true;
   }
+  case TargetLoweringBase::AtomicExpansionKind::CmpArithIntrinsic: {
+    TLI->emitCmpArithAtomicRMWIntrinsic(AI);
+    return true;
+  }
   case TargetLoweringBase::AtomicExpansionKind::NotAtomic:
     return lowerAtomicRMWInst(AI);
   case TargetLoweringBase::AtomicExpansionKind::Expand:
     TLI->emitExpandAtomicRMW(AI);
     return true;
   default:
     llvm_unreachable("Unhandled case in tryExpandAtomicRMW");
   }

llvm/lib/Target/X86/X86ISelLowering.h

private:
     shouldExpandAtomicLoadInIR(LoadInst *LI) const override;
     TargetLoweringBase::AtomicExpansionKind
     shouldExpandAtomicStoreInIR(StoreInst *SI) const override;
     TargetLoweringBase::AtomicExpansionKind
     shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const override;
     TargetLoweringBase::AtomicExpansionKind
     shouldExpandLogicAtomicRMWInIR(AtomicRMWInst *AI) const;
     void emitBitTestAtomicRMWIntrinsic(AtomicRMWInst *AI) const override;
+    void emitCmpArithAtomicRMWIntrinsic(AtomicRMWInst *AI) const override;
 
     LoadInst *
     lowerIdempotentRMWIntoFencedLoad(AtomicRMWInst *AI) const override;
 
     bool lowerAtomicStoreAsStoreSDNode(const StoreInst &SI) const override;
     bool lowerAtomicLoadAsLoadSDNode(const LoadInst &LI) const override;
 
     bool needsCmpXchgNb(Type *MemType) const;

llvm/lib/Target/X86/X86ISelLowering.cpp

if (!IntrData) {
     }
     case Intrinsic::x86_aadd32:
     case Intrinsic::x86_aadd64:
     case Intrinsic::x86_aand32:
     case Intrinsic::x86_aand64:
     case Intrinsic::x86_aor32:
     case Intrinsic::x86_aor64:
     case Intrinsic::x86_axor32:
-    case Intrinsic::x86_axor64: {
+    case Intrinsic::x86_axor64:
+    case Intrinsic::x86_atomic_add_cc:
+    case Intrinsic::x86_atomic_sub_cc: {
       Info.opc = ISD::INTRINSIC_W_CHAIN;
       Info.ptrVal = I.getArgOperand(0);
       unsigned Size = I.getArgOperand(1)->getType()->getScalarSizeInBits();
       Info.memVT = EVT::getIntegerVT(I.getType()->getContext(), Size);
       Info.align = Align(Size);
       Info.flags |= MachineMemOperand::MOLoad | MachineMemOperand::MOStore |
                     MachineMemOperand::MOVolatile;
       return true;
case Intrinsic::x86_axor64: {
       case Intrinsic::x86_axor64:
         Opc = X86ISD::AXOR;
         break;
       }
       MachineMemOperand *MMO = cast<MemSDNode>(Op)->getMemOperand();
       return DAG.getMemIntrinsicNode(Opc, DL, Op->getVTList(),
                                      {Chain, Op1, Op2}, VT, MMO);
     }
+    case Intrinsic::x86_atomic_add_cc:
+    case Intrinsic::x86_atomic_sub_cc: {
+      SDLoc DL(Op);
+      SDValue Chain = Op.getOperand(0);
+      SDValue Op1 = Op.getOperand(2);
+      SDValue Op2 = Op.getOperand(3);
+      X86::CondCode CC = (X86::CondCode)Op.getConstantOperandVal(4);
+      MVT VT = Op2.getSimpleValueType();
+      unsigned Opc = 0;
+      switch (IntNo) {
+      default:
+        llvm_unreachable("Unknown Intrinsic");
+      case Intrinsic::x86_atomic_add_cc:
+        Opc = X86ISD::LADD;
+        break;
+      case Intrinsic::x86_atomic_sub_cc:
+        Opc = X86ISD::LSUB;
+        break;
+      }
+      MachineMemOperand *MMO = cast<MemIntrinsicSDNode>(Op)->getMemOperand();
+      SDValue LockArith =
+          DAG.getMemIntrinsicNode(Opc, DL, DAG.getVTList(MVT::i32, MVT::Other),
+                                  {Chain, Op1, Op2}, VT, MMO);
+      Chain = LockArith.getValue(1);
+      return DAG.getMergeValues({getSETCC(CC, LockArith, DL, DAG), Chain}, DL);
+    }
     }
     return SDValue();
   }
 
   SDLoc dl(Op);
   switch(IntrData->Type) {
   default: llvm_unreachable("Unknown Intrinsic Type");
   case RDSEED:
void X86TargetLowering::emitBitTestAtomicRMWIntrinsic(AtomicRMWInst *AI) const {
   Value *Addr = Builder.CreatePointerCast(AI->getPointerOperand(),
                                           Type::getInt8PtrTy(Ctx));
   Value *Result = Builder.CreateCall(BitTest, {Addr, Builder.getInt8(Imm)});
   I->replaceAllUsesWith(Result);
   I->eraseFromParent();
   AI->eraseFromParent();
 }
 
+static bool shouldExpandCmpArithRMWInIR(AtomicRMWInst *AI) {
+  using namespace llvm::PatternMatch;
+  if (!AI->hasOneUse())
+    return false;
+
+  Value *Op = AI->getOperand(1);
+  ICmpInst::Predicate Pred;
+  Instruction *I = AI->user_back();
+  AtomicRMWInst::BinOp Opc = AI->getOperation();
+  if (Opc == AtomicRMWInst::Add) {
+    if (match(I, m_c_ICmp(Pred, m_Sub(m_ZeroInt(), m_Specific(Op)), m_Value())))
+      return Pred == CmpInst::ICMP_EQ;
+    if (match(I, m_c_Add(m_Specific(Op), m_Value())) && I->hasOneUse() &&
+        match(I->user_back(), m_ICmp(Pred, m_Value(), m_ZeroInt())))
+      return Pred == CmpInst::ICMP_SLT;
+    return false;
+  }
+  if (Opc == AtomicRMWInst::Sub) {
+    if (match(I, m_c_ICmp(Pred, m_Specific(Op), m_Value())))
+      return Pred == CmpInst::ICMP_EQ;
+    if (match(I, m_Sub(m_Value(), m_Specific(Op))) && I->hasOneUse() &&
+        match(I->user_back(), m_ICmp(Pred, m_Value(), m_ZeroInt())))
+      return Pred == CmpInst::ICMP_SLT;
+    return false;
+  }
+
+  return false;
+}
+
+void X86TargetLowering::emitCmpArithAtomicRMWIntrinsic(
+    AtomicRMWInst *AI) const {
+  IRBuilder<> Builder(AI);
+  Instruction *TempI = nullptr;
+  LLVMContext &Ctx = AI->getContext();
+  ICmpInst *ICI = dyn_cast<ICmpInst>(AI->user_back());
+  if (!ICI) {
+    TempI = AI->user_back();
+    ICI = cast<ICmpInst>(TempI->user_back());
+  }
+  X86::CondCode CC =
+      ICI->getPredicate() == CmpInst::ICMP_EQ ? X86::COND_E : X86::COND_S;
+  Intrinsic::ID IID = Intrinsic::not_intrinsic;
+  switch (AI->getOperation()) {
+  default:
+    llvm_unreachable("Unknown atomic operation");
+  case AtomicRMWInst::Add:
+    IID = Intrinsic::x86_atomic_add_cc;
+    break;
+  case AtomicRMWInst::Sub:
+    IID = Intrinsic::x86_atomic_sub_cc;
+    break;
+  }
+  Function *CmpArith =
+      Intrinsic::getDeclaration(AI->getModule(), IID, AI->getType());
+  Value *Call = Builder.CreateCall(CmpArith, {AI->getPointerOperand(),
+                                              AI->getValOperand(),
+                                              Builder.getInt32((unsigned)CC)});
+  Value *Result = Builder.CreateTrunc(Call, Type::getInt1Ty(Ctx));
+  ICI->replaceAllUsesWith(Result);
+  ICI->eraseFromParent();
+  if (TempI)
+    TempI->eraseFromParent();
+  AI->eraseFromParent();
+}
+
 TargetLowering::AtomicExpansionKind
 X86TargetLowering::shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const {
   unsigned NativeWidth = Subtarget.is64Bit() ? 64 : 32;
   Type *MemType = AI->getType();
 
   // If the operand is too big, we must see if cmpxchg8/16b is available
   // and default to library calls otherwise.
   if (MemType->getPrimitiveSizeInBits() > NativeWidth) {
     return needsCmpXchgNb(MemType) ? AtomicExpansionKind::CmpXChg
                                    : AtomicExpansionKind::None;
   }
 
   AtomicRMWInst::BinOp Op = AI->getOperation();
   switch (Op) {
   default:
     llvm_unreachable("Unknown atomic operation");
   case AtomicRMWInst::Xchg:
+    return AtomicExpansionKind::None;
   case AtomicRMWInst::Add:
   case AtomicRMWInst::Sub:
-    // It's better to use xadd, xsub or xchg for these in all cases.
+    if (shouldExpandCmpArithRMWInIR(AI))
+      return AtomicExpansionKind::CmpArithIntrinsic;
+    // It's better to use xadd, xsub or xchg for these in other cases.
     return AtomicExpansionKind::None;
   case AtomicRMWInst::Or:
   case AtomicRMWInst::And:
   case AtomicRMWInst::Xor:
     return shouldExpandLogicAtomicRMWInIR(AI);
   case AtomicRMWInst::Nand:
   case AtomicRMWInst::Max:
   case AtomicRMWInst::Min:

llvm/test/CodeGen/X86/pr37025.ll

 
 8: ; preds = %7, %2
   ret void
 }
 
 define void @test_dec_select_commute(ptr nocapture %0, ptr readnone %1) {
 ; CHECK-LABEL: test_dec_select_commute:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movq $-1, %rax
-; CHECK-NEXT:    lock xaddq %rax, (%rdi)
+; CHECK-NEXT:    lock decq (%rdi)
+; CHECK-NEXT:    sete %al
 ; CHECK-NEXT:    testq %rsi, %rsi
 ; CHECK-NEXT:    je .LBB1_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    cmpq $1, %rax
-; CHECK-NEXT:    jne .LBB1_2
+; CHECK-NEXT:    testb %al, %al
+; CHECK-NEXT:    je .LBB1_2
 ; CHECK-NEXT:  # %bb.3:
 ; CHECK-NEXT:    jmp func2 # TAILCALL
 ; CHECK-NEXT:  .LBB1_2:
 ; CHECK-NEXT:    retq
   %3 = atomicrmw sub ptr %0, i64 1 seq_cst
   %4 = icmp eq i64 %3, 1
   %5 = icmp ne ptr %1, null
   %6 = select i1 %5, i1 %4, i1 false
   br i1 %6, label %7, label %8
 
 7: ; preds = %2
   tail call void @func2()
   br label %8
 
 8: ; preds = %7, %2
   ret void
 }
 
 define void @test_dec_and(ptr nocapture %0, ptr readnone %1) {
 ; CHECK-LABEL: test_dec_and:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movq $-1, %rax
-; CHECK-NEXT:    lock xaddq %rax, (%rdi)
+; CHECK-NEXT:    lock decq (%rdi)
+; CHECK-NEXT:    sete %al
 ; CHECK-NEXT:    testq %rsi, %rsi
 ; CHECK-NEXT:    je .LBB2_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    cmpq $1, %rax
-; CHECK-NEXT:    jne .LBB2_2
+; CHECK-NEXT:    testb %al, %al
+; CHECK-NEXT:    je .LBB2_2
 ; CHECK-NEXT:  # %bb.3:
 ; CHECK-NEXT:    jmp func2 # TAILCALL
 ; CHECK-NEXT:  .LBB2_2:
 ; CHECK-NEXT:    retq
   %3 = atomicrmw sub ptr %0, i64 1 seq_cst
   %4 = icmp eq i64 %3, 1
   %5 = icmp ne ptr %1, null
   %6 = and i1 %5, %4

llvm/test/CodeGen/X86/pr58685.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s
+
+define dso_local noundef zeroext i1 @_Z13lock_add_seteRSt6atomicIjEj(ptr nocapture noundef nonnull align 4 dereferenceable(4) %0, i32 noundef %1) local_unnamed_addr #0 {
+; CHECK-LABEL: _Z13lock_add_seteRSt6atomicIjEj:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock addl %esi, (%rdi)
+; CHECK-NEXT:    sete %al
+; CHECK-NEXT:    retq
+  %3 = atomicrmw add ptr %0, i32 %1 seq_cst, align 4
+  %4 = sub i32 0, %1
+  %5 = icmp eq i32 %3, %4
+  ret i1 %5
+}
+
+define dso_local noundef zeroext i1 @_Z13lock_add_setsRSt6atomicIjEj(ptr nocapture noundef nonnull align 4 dereferenceable(4) %0, i32 noundef %1) local_unnamed_addr #0 {
+; CHECK-LABEL: _Z13lock_add_setsRSt6atomicIjEj:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock addl %esi, (%rdi)
+; CHECK-NEXT:    sets %al
+; CHECK-NEXT:    retq
+  %3 = atomicrmw add ptr %0, i32 %1 seq_cst, align 4
+  %4 = add i32 %3, %1
+  %5 = icmp slt i32 %4, 0
+  ret i1 %5
+}
+
+define dso_local noundef zeroext i1 @_Z13lock_sub_seteRSt6atomicIjEj(ptr nocapture noundef nonnull align 4 dereferenceable(4) %0, i32 noundef %1) local_unnamed_addr #0 {
+; CHECK-LABEL: _Z13lock_sub_seteRSt6atomicIjEj:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock subl %esi, (%rdi)
+; CHECK-NEXT:    sete %al
+; CHECK-NEXT:    retq
+  %3 = atomicrmw sub ptr %0, i32 %1 seq_cst, align 4
+  %4 = icmp eq i32 %3, %1
+  ret i1 %4
+}
+
+define dso_local noundef zeroext i1 @_Z13lock_sub_setsRSt6atomicIjEj(ptr nocapture noundef nonnull align 4 dereferenceable(4) %0, i32 noundef %1) local_unnamed_addr #0 {
+; CHECK-LABEL: _Z13lock_sub_setsRSt6atomicIjEj:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock subl %esi, (%rdi)
+; CHECK-NEXT:    sets %al
+; CHECK-NEXT:    retq
+  %3 = atomicrmw sub ptr %0, i32 %1 seq_cst, align 4
+  %4 = sub i32 %3, %1
+  %5 = icmp slt i32 %4, 0
+  ret i1 %5
+}
+
+attributes #0 = { mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite) uwtable "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }