[PATCH] D15592: [LICM] Make store promotion work in the face of unordered atomics

[Hal Finkel via llvm-commits]

hfinkel added a comment.

In http://reviews.llvm.org/D15592#343574, @reames wrote:

> In http://reviews.llvm.org/D15592#342294, @hfinkel wrote:
>
> > > Promoting the non-atomic accesses to atomic would be incorrect.
> >
> >
> > Why?
>
>
> Per the example I gave, we might not be able to lower a 128 bit atomic, and the code might be conditional on that fact.  Inserting an unconditional i128 atomic which can't be lowered which wasn't in the original code in that case seems "questionable".  I didn't have a need to do this and the semantics seemed a bit unclear, so it seemed better to avoid.


Fair enough, and I'm fine with not doing this. I don't understand your example, however, because if we can't lower the atomic, then the compilation will fail regardless of whether or not the atomic is conditionally executed. We still need to generate code for it if it is present.

> 

> 

> > Philip, can you comment on the original use cases here? Are there cases where indefinitely postponing the write (because, say, the loop ended up not terminating) would be acceptable?

> 

> 

> As I said previous, I consider most of JFs comments to be wildly off topic.  Particularly, the "unordered" ordering is not related to C++ at all.  As specifically documented in the LangRef: "This is intended to provide a guarantee strong enough to model Java’s non-volatile shared variables. "  This is exactly the purpose I'm using it for.  To the best of my knowledge (which admittedly isn't that extensive here), there is no way to generate an "unordered" instruction from C++ code.


I agree, the C++ rules here are not strictly relevant.

> On the topic of infinite loops, I could go either way.  In practice, any infinite loop will have a safepoint or deoptimization point within it, so the loop promotion couldn't trigger anyways.  Given that, I don't see any reason why we'd need to treat potentially infinite loops specially here.  (Again, C++'s rules are not relevant.)


I understand, and the fact that this can't come up in your environment is interesting. However, from LLVM's perspective, we still need to decide on the semantics here.

> (Hoping to get back to this line of work soon.  Distracted with higher priority tasks at the moment.)





http://reviews.llvm.org/D15592