Skip to content

Commit

Permalink
Do not emit fences when peephole optimizing struct RMWs (#7281)
Browse files Browse the repository at this point in the history 
We previously reasoned that we had to emit fences when optimizing
sequentially consistent RMWs to unshared memory to preserve the RMWs'
effect on the total order of sequentially consistent operations.
However, that total order alone is insufficient to force any read event
to observe some other write event; that requires a synchronization edge
to be formed via an atomic write and read pair. RMWs to unshared memory
can never be part of such a synchronization edge, so removing them
cannot introduce new behaviors. Improve our optimization to stop
emitting the unnecessary fences.
  • Loading branch information
@tlively
tlively authored Feb 7, 2025
1 parent 088b103 commit 1de13c3
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 29 deletions.
12 changes: 0 additions & 12 deletions src/passes/OptimizeInstructions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1981,12 +1981,6 @@ struct OptimizeInstructions
newVal,
MemoryOrder::Unordered));

// We must maintain this operation's effect on the global order of seqcst
// operations.
if (curr->order == MemoryOrder::SeqCst) {
block->list.push_back(builder.makeAtomicFence());
}

block->list.push_back(builder.makeLocalGet(result, curr->type));
block->type = curr->type;
replaceCurrent(block);
Expand Down Expand Up @@ -2061,12 +2055,6 @@ struct OptimizeInstructions
builder.makeLocalGet(replacement, curr->type),
MemoryOrder::Unordered)));

// We must maintain this operation's effect on the global order of seqcst
// operations.
if (curr->order == MemoryOrder::SeqCst) {
block->list.push_back(builder.makeAtomicFence());
}

block->list.push_back(builder.makeLocalGet(result, curr->type));
block->type = curr->type;
replaceCurrent(block);
Expand Down
18 changes: 1 addition & 17 deletions test/lit/passes/optimize-instructions-struct-rmw.wast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-add-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -714,7 +713,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-sub-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -746,7 +744,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-and-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -778,7 +775,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-or-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -810,7 +806,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-xor-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -839,7 +834,6 @@
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-xchg-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -879,7 +873,6 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
(func $cmpxchg-i32-lower (param (ref null $unshared-i32)) (result i32)
Expand Down Expand Up @@ -912,7 +905,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-add-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -944,7 +936,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-sub-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -976,7 +967,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-and-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -1008,7 +998,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-or-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -1040,7 +1029,6 @@
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-xor-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -1069,7 +1057,6 @@
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-xchg-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -1109,7 +1096,6 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
(func $cmpxchg-i64-lower (param (ref null $unshared-i64)) (result i64)
Expand Down Expand Up @@ -1139,7 +1125,6 @@
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-xchg-ref-lower (param (ref null $unshared-struct)) (result (ref null $unshared-struct))
Expand Down Expand Up @@ -1179,7 +1164,6 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (atomic.fence)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
(func $cmpxchg-ref-lower (param (ref null $unshared-struct)) (result (ref null $unshared-struct))
Expand Down Expand Up @@ -1215,7 +1199,7 @@
;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
(func $rmw-add-i32-acqrel (param (ref null $unshared-i32)) (result i32)
;; Check that the lowering of an acqrel RMW does not have a fence.
;; Check that the lowering of an acqrel RMW works the same way.
(struct.atomic.rmw.add acqrel acqrel $unshared-i32 0
(local.get 0)
(i32.const 1)
Expand Down

0 comments on commit 1de13c3

Please sign in to comment.