[X86] Don't transform atomic-load-add into an inc/dec when inc/dec is slow

[morisset]

Summary

I had to move code around and turn a static function into a method to get
access to Subtarget, but the only "real" change is the extra check for
!Subtarget->slowIncDec().

[jfb]

lib/Target/X86/X86ISelDAGToDAG.cpp

1711

Can you keep this static and pass in the Subtarget?

[morisset]

lib/Target/X86/X86ISelDAGToDAG.cpp

1711

I could easily do that. I don't see how exactly it would be better, could you explain it to me ?

[jfb]

lib/Target/X86/X86ISelDAGToDAG.cpp

1711

It makes it easier to understand the side-effects that the function can have: it can't access or modify any of the class' internals. It makes it harder to inadvertently add things like this to the function: you have to pass them in as parameters.

[morisset]

Follow jfb suggestion.

[jfb]

lgtm

[morisset]

r219357

lib/Target/X86/X86ISelDAGToDAG.cpp

 // translations, such as from atomic-load-add to lock-sub. The return value is
 // one of the following 3 cases:
 // + target-constant, the operand could be supported as a target constant.
 // + empty, the operand is not needed any more with the new op selected.
 // + non-empty, otherwise.
 static SDValue getAtomicLoadArithTargetConstant(SelectionDAG *CurDAG,
                                                 SDLoc dl,
                                                 enum AtomicOpc &Op, MVT NVT,
-                                                SDValue Val) {
+                                                SDValue Val,
+                                                const X86Subtarget *Subtarget) {
   if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Val)) {
     int64_t CNVal = CN->getSExtValue();
     // Quit if not 32-bit imm.
     if ((int32_t)CNVal != CNVal)
       return Val;
     // Quit if INT32_MIN: it would be negated as it is negative and overflow,
     // producing an immediate that does not fit in the 32 bits available for
     // an immediate operand to sub. However, it still fits in 32 bits for the
     // add (since it is not negated) so we can return target-constant.
     if (CNVal == INT32_MIN)
       return CurDAG->getTargetConstant(CNVal, NVT);
     // For atomic-load-add, we could do some optimizations.
     if (Op == ADD) {
       // Translate to INC/DEC if ADD by 1 or -1.
-      if ((CNVal == 1) || (CNVal == -1)) {
+      if (((CNVal == 1) || (CNVal == -1)) && !Subtarget->slowIncDec()) {
         Op = (CNVal == 1) ? INC : DEC;
         // No more constant operand after being translated into INC/DEC.
         return SDValue();
       }
       // Translate to SUB if ADD by negative value.
       if (CNVal < 0) {
         Op = SUB;
         CNVal = -CNVal;
switch (Node->getOpcode()) {
     case ISD::ATOMIC_LOAD_XOR:
       Op = XOR;
       break;
     case ISD::ATOMIC_LOAD_ADD:
       Op = ADD;
       break;
   }
 
-  Val = getAtomicLoadArithTargetConstant(CurDAG, dl, Op, NVT, Val);
+  Val = getAtomicLoadArithTargetConstant(CurDAG, dl, Op, NVT, Val, Subtarget);
   bool isUnOp = !Val.getNode();
   bool isCN = Val.getNode() && (Val.getOpcode() == ISD::TargetConstant);
 
   unsigned Opc = 0;
   switch (NVT.SimpleTy) {
     default: return nullptr;
     case MVT::i8:
       if (isCN)

test/CodeGen/X86/atomic_add.ll

 ; RUN: llc < %s -march=x86-64 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -march=x86-64 -mattr=slow-incdec -verify-machineinstrs | FileCheck %s --check-prefix SLOW_INC
 
 ; rdar://7103704
 
 define void @sub1(i32* nocapture %p, i32 %v) nounwind ssp {
 entry:
 ; CHECK-LABEL: sub1:
 ; CHECK: subl
   %0 = atomicrmw sub i32* %p, i32 %v monotonic
   ret void
 }
 
 define void @inc4(i64* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: inc4:
 ; CHECK: incq
+; SLOW_INC-LABEL: inc4:
+; SLOW_INC-NOT: incq
   %0 = atomicrmw add i64* %p, i64 1 monotonic
   ret void
 }
 
 define void @add8(i64* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: add8:
 ; CHECK: addq $2
; CHECK: addq
   %1 = atomicrmw add i64* %p, i64 %0 monotonic
   ret void
 }
 
 define void @inc3(i8* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: inc3:
 ; CHECK: incb
+; SLOW_INC-LABEL: inc3:
+; SLOW_INC-NOT: incb
   %0 = atomicrmw add i8* %p, i8 1 monotonic
   ret void
 }
 
 define void @add7(i8* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: add7:
 ; CHECK: addb $2
; CHECK: addb
   %1 = atomicrmw add i8* %p, i8 %0 monotonic
   ret void
 }
 
 define void @inc2(i16* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: inc2:
 ; CHECK: incw
+; SLOW_INC-LABEL: inc2:
+; SLOW_INC-NOT: incw
   %0 = atomicrmw add i16* %p, i16 1 monotonic
   ret void
 }
 
 define void @add6(i16* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: add6:
 ; CHECK: addw $2
; CHECK: addw
   %1 = atomicrmw add i16* %p, i16 %0 monotonic
   ret void
 }
 
 define void @inc1(i32* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: inc1:
 ; CHECK: incl
+; SLOW_INC-LABEL: inc1:
+; SLOW_INC-NOT: incl
   %0 = atomicrmw add i32* %p, i32 1 monotonic
   ret void
 }
 
 define void @add5(i32* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: add5:
 ; CHECK: addl $2
   %0 = atomicrmw add i32* %p, i32 2 monotonic
   ret void
 }
 
 define void @add1(i32* nocapture %p, i32 %v) nounwind ssp {
 entry:
 ; CHECK-LABEL: add1:
 ; CHECK: addl
   %0 = atomicrmw add i32* %p, i32 %v monotonic
   ret void
 }
 
 define void @dec4(i64* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: dec4:
 ; CHECK: decq
+; SLOW_INC-LABEL: dec4:
+; SLOW_INC-NOT: decq
   %0 = atomicrmw sub i64* %p, i64 1 monotonic
   ret void
 }
 
 define void @sub8(i64* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: sub8:
 ; CHECK: subq $2
; CHECK: subq
   %1 = atomicrmw sub i64* %p, i64 %0 monotonic
   ret void
 }
 
 define void @dec3(i8* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: dec3:
 ; CHECK: decb
+; SLOW_INC-LABEL: dec3:
+; SLOW_INC-NOT: decb
   %0 = atomicrmw sub i8* %p, i8 1 monotonic
   ret void
 }
 
 define void @sub7(i8* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: sub7:
 ; CHECK: subb $2
; CHECK: subb
   %1 = atomicrmw sub i8* %p, i8 %0 monotonic
   ret void
 }
 
 define void @dec2(i16* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: dec2:
 ; CHECK: decw
+; SLOW_INC-LABEL: dec2:
+; SLOW_INC-NOT: decw
   %0 = atomicrmw sub i16* %p, i16 1 monotonic
   ret void
 }
 
 define void @sub6(i16* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: sub6:
 ; CHECK: subw $2
; CHECK: subw
   %1 = atomicrmw sub i16* %p, i16 %0 monotonic
   ret void
 }
 
 define void @dec1(i32* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: dec1:
 ; CHECK: decl
+; SLOW_INC-LABEL: dec1:
+; SLOW_INC-NOT: decl
   %0 = atomicrmw sub i32* %p, i32 1 monotonic
   ret void
 }
 
 define void @sub5(i32* nocapture %p) nounwind ssp {
 entry:
 ; CHECK-LABEL: sub5:
 ; CHECK: subl $2
   %0 = atomicrmw sub i32* %p, i32 2 monotonic
   ret void
 }