[PATCH] D26348: Allow convergent attribute for function arguments

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D26348#693554, @dark_sylinc wrote:

> In https://reviews.llvm.org/D26348#693537, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D26348#693528, @hakzsam wrote:
> >
> > > This patch should also fix https://bugs.freedesktop.org/show_bug.cgi?id=99484
> >
> >
> > Please read above my comment about this bug and how it can be fixed without changing LLVM.
>
>
> While true, making texture calls have unknown behavior forces the compiler to prevent reordering at all... on an architecture that heavily relies on latency hiding (both horizontal i.e. wave occupancy and vertical i.e. instruction reordering).


Right now, frontends are given a choice between performance and correctness... 
My point is that one can't choose performance and sacrifices knowingly correctness, and then complains about a LLVM bug that need to be fixed. The bug is in the shader compiler in the first place!

I perfectly understand how having a solution to enable these optimizations is very important for shader compilers.
It is likely that if I was in charge of such a shader compiler, I'd try to explore how to express my intrinsics differently such that I can get "most" optimizations  (i.e. not having to say to LLVM that this call can do anything).

Example to think about: can token help? Could the intrinsic be modeled by taking a pointer to a memory location instead of a SSA value? (there are finer grain way to express the effect on a pointer argument, and it wouldn't break SSA).

> Btw. I've been running Mesa radeonsi with this patch on top of LLVM 5.0 for my daily work and leisure time for almost a month now, and haven't encountered any issue.

Great. But be aware that anecdotal data are unlikely to expedite a review here. What is needed is someone to spend time to find a solution to and nail the issues discussed in LangRef above.


https://reviews.llvm.org/D26348