[llvm] 6b85fa6 - [InstCombine] Don't optimize idempotent `atomicrmw <op>, 0` into `load atomic`
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 19 01:20:33 PST 2023
Author: Quentin Colombet
Date: 2023-01-19T10:04:07+01:00
New Revision: 6b85fa6d81b9f50d7a1c347dfd467c9ab01c4a5d
URL: https://github.com/llvm/llvm-project/commit/6b85fa6d81b9f50d7a1c347dfd467c9ab01c4a5d
DIFF: https://github.com/llvm/llvm-project/commit/6b85fa6d81b9f50d7a1c347dfd467c9ab01c4a5d.diff
LOG: [InstCombine] Don't optimize idempotent `atomicrmw <op>, 0` into `load atomic`
Turning idempotent `atomicrmw`s into `load atomic` is perfectly legal
with respect to how the loading happens, but it may not be legal for the
whole program semantic.
Indeed, this optimization removes a store that may have some effects on
the legality of other optimizations.
Essentially, we lose some information and depending on the backend
it may or may not produce incorrect code, so don't do it!
This fixes llvm.org/PR56450.
Differential Revision: https://reviews.llvm.org/D141277
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
llvm/test/Transforms/InstCombine/atomicrmw.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index 06d8fb2d474dc..e73667f9c02ea 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -151,13 +151,5 @@ Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
return replaceOperand(RMWI, 1, ConstantFP::getNegativeZero(RMWI.getType()));
}
- // Check if the required ordering is compatible with an atomic load.
- if (Ordering != AtomicOrdering::Acquire &&
- Ordering != AtomicOrdering::Monotonic)
- return nullptr;
-
- LoadInst *Load = new LoadInst(RMWI.getType(), RMWI.getPointerOperand(), "",
- false, DL.getABITypeAlign(RMWI.getType()),
- Ordering, RMWI.getSyncScopeID());
- return Load;
+ return nullptr;
}
diff --git a/llvm/test/Transforms/InstCombine/atomicrmw.ll b/llvm/test/Transforms/InstCombine/atomicrmw.ll
index 7da822b00a635..d2789e4a636d9 100644
--- a/llvm/test/Transforms/InstCombine/atomicrmw.ll
+++ b/llvm/test/Transforms/InstCombine/atomicrmw.ll
@@ -3,12 +3,15 @@
; Check that we can replace `atomicrmw <op> LHS, 0` with `load atomic LHS`.
; This is possible when:
-; - <op> LHS, 0 == LHS
-; - the ordering of atomicrmw is compatible with a load (i.e., no release semantic)
+; Check that we don't replace `atomicrmw <op> LHS, 0` with `load atomic LHS`.
+; Doing that would lose the store semantic of the `atomicrmw` operation.
+; This may enable some other optimizations that would otherwise be illegal when
+; the store semantic was present (e.g., like dropping a fence).
+; Idempotent atomicrmw are still canonicalized.
define i32 @atomic_add_zero(ptr %addr) {
; CHECK-LABEL: @atomic_add_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i32, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i32 0 monotonic, align 4
; CHECK-NEXT: ret i32 [[RES]]
;
%res = atomicrmw add ptr %addr, i32 0 monotonic
@@ -17,70 +20,77 @@ define i32 @atomic_add_zero(ptr %addr) {
define i32 @atomic_or_zero(ptr %addr) {
; CHECK-LABEL: @atomic_or_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i32, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i32 0 monotonic, align 4
; CHECK-NEXT: ret i32 [[RES]]
;
%res = atomicrmw add ptr %addr, i32 0 monotonic
ret i32 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i32 @atomic_sub_zero(ptr %addr) {
; CHECK-LABEL: @atomic_sub_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i32, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i32 0 monotonic, align 4
; CHECK-NEXT: ret i32 [[RES]]
;
%res = atomicrmw sub ptr %addr, i32 0 monotonic
ret i32 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i32 @atomic_and_allones(ptr %addr) {
; CHECK-LABEL: @atomic_and_allones(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i32, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i32 0 monotonic, align 4
; CHECK-NEXT: ret i32 [[RES]]
;
%res = atomicrmw and ptr %addr, i32 -1 monotonic
ret i32 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i32 @atomic_umin_uint_max(ptr %addr) {
; CHECK-LABEL: @atomic_umin_uint_max(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i32, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i32 0 monotonic, align 4
; CHECK-NEXT: ret i32 [[RES]]
;
%res = atomicrmw umin ptr %addr, i32 -1 monotonic
ret i32 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i32 @atomic_umax_zero(ptr %addr) {
; CHECK-LABEL: @atomic_umax_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i32, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i32 0 monotonic, align 4
; CHECK-NEXT: ret i32 [[RES]]
;
%res = atomicrmw umax ptr %addr, i32 0 monotonic
ret i32 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i8 @atomic_min_smax_char(ptr %addr) {
; CHECK-LABEL: @atomic_min_smax_char(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i8, ptr [[ADDR:%.*]] monotonic, align 1
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i8 0 monotonic, align 1
; CHECK-NEXT: ret i8 [[RES]]
;
%res = atomicrmw min ptr %addr, i8 127 monotonic
ret i8 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i8 @atomic_max_smin_char(ptr %addr) {
; CHECK-LABEL: @atomic_max_smin_char(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i8, ptr [[ADDR:%.*]] monotonic, align 1
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i8 0 monotonic, align 1
; CHECK-NEXT: ret i8 [[RES]]
;
%res = atomicrmw max ptr %addr, i8 -128 monotonic
ret i8 %res
}
+; Idempotent atomicrmw are still canonicalized.
define float @atomic_fsub_zero(ptr %addr) {
; CHECK-LABEL: @atomic_fsub_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic float, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw fadd ptr [[ADDR:%.*]], float -0.000000e+00 monotonic, align 4
; CHECK-NEXT: ret float [[RES]]
;
%res = atomicrmw fsub ptr %addr, float 0.0 monotonic
@@ -89,13 +99,14 @@ define float @atomic_fsub_zero(ptr %addr) {
define float @atomic_fadd_zero(ptr %addr) {
; CHECK-LABEL: @atomic_fadd_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic float, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw fadd ptr [[ADDR:%.*]], float -0.000000e+00 monotonic, align 4
; CHECK-NEXT: ret float [[RES]]
;
%res = atomicrmw fadd ptr %addr, float -0.0 monotonic
ret float %res
}
+; Idempotent atomicrmw are still canonicalized.
define float @atomic_fsub_canon(ptr %addr) {
; CHECK-LABEL: @atomic_fsub_canon(
; CHECK-NEXT: [[RES:%.*]] = atomicrmw fadd ptr [[ADDR:%.*]], float -0.000000e+00 release, align 4
@@ -126,9 +137,10 @@ define i64 @atomic_sub_zero_volatile(ptr %addr) {
; Check that the transformation properly preserve the syncscope.
+; Idempotent atomicrmw are still canonicalized.
define i16 @atomic_syncscope(ptr %addr) {
; CHECK-LABEL: @atomic_syncscope(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i16, ptr [[ADDR:%.*]] syncscope("some_syncscope") acquire, align 2
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i16 0 syncscope("some_syncscope") acquire, align 2
; CHECK-NEXT: ret i16 [[RES]]
;
%res = atomicrmw or ptr %addr, i16 0 syncscope("some_syncscope") acquire
@@ -157,9 +169,10 @@ define i16 @atomic_add_non_zero(ptr %addr) {
ret i16 %res
}
+; Idempotent atomicrmw are still canonicalized.
define i16 @atomic_xor_zero(ptr %addr) {
; CHECK-LABEL: @atomic_xor_zero(
-; CHECK-NEXT: [[RES:%.*]] = load atomic i16, ptr [[ADDR:%.*]] monotonic, align 2
+; CHECK-NEXT: [[RES:%.*]] = atomicrmw or ptr [[ADDR:%.*]], i16 0 monotonic, align 2
; CHECK-NEXT: ret i16 [[RES]]
;
%res = atomicrmw xor ptr %addr, i16 0 monotonic
More information about the llvm-commits
mailing list