[PATCH] Remove ISD::MEMBARRIER node?

Rafael EspĂ­ndola rafael.espindola at gmail.com
Sat Apr 20 04:51:58 PDT 2013


LGTM.

It lives is svn history in case anyone wants in back.

On 20 April 2013 05:47, Tim Northover <t.p.northover at gmail.com> wrote:
>> (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.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



More information about the llvm-commits mailing list