[PATCH] D38392: Disallow sinking of unordered atomic loads into loops

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In https://reviews.llvm.org/D38392#885147, @danielcdh wrote:

> In https://reviews.llvm.org/D38392#885134, @reames wrote:
>
> > In https://reviews.llvm.org/D38392#885108, @danielcdh wrote:
> >
> > > I see your point now. My concern is performance: if we allow hoisting of atomic load, but not allow sinking it, we may end up with bad performance as we may have too much redundant atomic loads in the preheader. Any suggestions on how to solve that?
> >
> >
> > Not really.  I can say that we've been running performance tests for months in this configuration (LICM hoisting unordered loads, no LoopSink) without noticing any problems.  I'm not immediately concerned.
>
>
> Sorry, not sure if I follow the logic, could you explain why you don't think this is a performance concern?


I acknowledge it's a potential issue.  I have no good ideas for a solution.  I don't believe it's a current problem and evidence to believe that it's relatively minor.  (i.e. we haven't seen it)


https://reviews.llvm.org/D38392