[PATCH] Remove ISD::MEMBARRIER node?

Tim Northover t.p.northover at gmail.com
Sat Apr 20 02:47:59 PDT 2013


> (N.b. I've left the ShouldFoldAtomicFences stuff in even though it's
> unused after this patch because I intend to work on a set of valid
> ATOMIC_FENCE combines this weekend).

Hmm, that plan doesn't seem to have worked so well. As far as I can
tell there's essentially no way to combine fences with surrounding
atomic operations within LLVM's defined concurrency semantics.

It comes down to the fact that synchronisation happens between two
operations on a specific memory location, so if the fence gets
combined with the "wrong" memory operation, it no longer provides the
needed ordering constraints. And, of course, there's no local way to
tell which operation(s) it is intended to provide synchronisation for
(in the worst case, it could even be multiple loads and stores
occurring in multiple different functions).

E.g.
T1:
---
fence release
store atomic M_1 monotonic

T2:
---
fence release
store atomic M_2 monotonic

T3:
---
load atomic M_1 monotonic
load atomic M_2 monotonic
fence acquire

T3's fence can synchronise with both T1 and T2, but combining it with
just one of the loads in T3 only guarantees synchronisation with one
of the threads. Combining it with both, to give:

T3:
---
load atomic M_1 acquire
load atomic M_2 acquire

also seems dodgy (a seq_cst fence would no longer be sequentially
consistent, I think), and probably not profitable even if it's OK.

In practice, I suspect at least some individual architectures *would*
permit the merge, but I don't think the generic DAGCombiner is the
place for such dodgy transformations. So the attached patch also
removes all references to the unused ShouldFoldAtomicFences.

It obviously depends on the original change I'm replying to (also
re-attached).

Tim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: remove-should-fold.diff
Type: application/octet-stream
Size: 4550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130420/39fd7d15/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: remove-membarrier.diff
Type: application/octet-stream
Size: 22518 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130420/39fd7d15/attachment-0001.obj>


More information about the llvm-commits mailing list