Index: lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp =================================================================== --- lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp +++ lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp @@ -54,8 +54,15 @@ if (!isIdempotentRMW(RMWI)) return nullptr; - // TODO: Canonicalize the operation for an idempotent operation we can't - // convert into a simple load. + // We chose to canonicalize all idempotent operations to an single + // operation code and constant. This makes it easier for the rest of the + // optimizer to match easily. The choice of or w/zero is arbitrary. + if (RMWI.getType()->isIntegerTy() && + RMWI.getOperation() != AtomicRMWInst::Or) { + RMWI.setOperation(AtomicRMWInst::Or); + RMWI.setOperand(1, ConstantInt::get(RMWI.getType(), 0)); + return &RMWI; + } // Volatile RMWs perform a load and a store, we cannot replace // this by just a load. Index: test/Transforms/InstCombine/atomicrmw.ll =================================================================== --- test/Transforms/InstCombine/atomicrmw.ll +++ test/Transforms/InstCombine/atomicrmw.ll @@ -69,8 +69,9 @@ ; Can't replace a volatile w/a load; this would eliminate a volatile store. +; We can canonicalize the opcode ; CHECK-LABEL: atomic_sub_zero_volatile -; CHECK-NEXT: %res = atomicrmw volatile sub i64* %addr, i64 0 acquire +; CHECK-NEXT: %res = atomicrmw volatile or i64* %addr, i64 0 acquire ; CHECK-NEXT: ret i64 %res define i64 @atomic_sub_zero_volatile(i64* %addr) { %res = atomicrmw volatile sub i64* %addr, i64 0 acquire @@ -87,14 +88,13 @@ ret i16 %res } -; Don't transform seq_cst ordering. ; By eliminating the store part of the atomicrmw, we would get rid of the -; release semantic, which is incorrect. -; CHECK-LABEL: atomic_or_zero_seq_cst +; release semantic, which is incorrect. We can canonicalize the oepration. +; CHECK-LABEL: atomic_seq_cst ; CHECK-NEXT: %res = atomicrmw or i16* %addr, i16 0 seq_cst ; CHECK-NEXT: ret i16 %res -define i16 @atomic_or_zero_seq_cst(i16* %addr) { - %res = atomicrmw or i16* %addr, i16 0 seq_cst +define i16 @atomic_seq_cst(i16* %addr) { + %res = atomicrmw add i16* %addr, i16 0 seq_cst ret i16 %res } @@ -117,21 +117,21 @@ } ; Check that the transformation does not apply when the ordering is -; incompatible with a load (release). -; CHECK-LABEL: atomic_or_zero_release +; incompatible with a load (release). Do canonicalize. +; CHECK-LABEL: atomic_release ; CHECK-NEXT: %res = atomicrmw or i16* %addr, i16 0 release ; CHECK-NEXT: ret i16 %res -define i16 @atomic_or_zero_release(i16* %addr) { - %res = atomicrmw or i16* %addr, i16 0 release +define i16 @atomic_release(i16* %addr) { + %res = atomicrmw sub i16* %addr, i16 0 release ret i16 %res } ; Check that the transformation does not apply when the ordering is -; incompatible with a load (acquire, release). -; CHECK-LABEL: atomic_or_zero_acq_rel +; incompatible with a load (acquire, release). Do canonicalize. +; CHECK-LABEL: atomic_acq_rel ; CHECK-NEXT: %res = atomicrmw or i16* %addr, i16 0 acq_rel ; CHECK-NEXT: ret i16 %res -define i16 @atomic_or_zero_acq_rel(i16* %addr) { - %res = atomicrmw or i16* %addr, i16 0 acq_rel +define i16 @atomic_acq_rel(i16* %addr) { + %res = atomicrmw xor i16* %addr, i16 0 acq_rel ret i16 %res }