[PATCH] D38113: OpenCL: Assume functions are convergent

2017-10-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision. arsenm added a comment. r315094 Comment at: test/CodeGenOpenCL/convergent.cl:130 +// CHECK: attributes #0 = { noinline norecurse nounwind " +// CHECK: attributes #1 = { {{[^}]*}}convergent{{[^}]*}} } +// CHECK: attributes #2 = { {{[^}]*}}convergent{

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-10-06 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D38113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-10-05 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 117855. arsenm added a comment. Check noduplicate https://reviews.llvm.org/D38113 Files: include/clang/Basic/LangOptions.h lib/CodeGen/CGCall.cpp test/CodeGenOpenCL/amdgpu-attrs.cl test/CodeGenOpenCL/convergent.cl Index: test/CodeGenOpenCL/convergen

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-27 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D38113#882187, @Anastasia wrote: > In https://reviews.llvm.org/D38113#878840, @jlebar wrote: > > > > ... > Yes, I see this sounds more reasonable indeed. Btw, currently LLVM can remove > `convergent` in some cases to recover the performance

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-27 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. In https://reviews.llvm.org/D38113#878840, @jlebar wrote: > > Yes, that's why if it would be responsibility of the kernel developer to > > specify this explicitly we could avoid this complications in the compiler. > > But if we add it into the language now we still ne

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-27 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. In https://reviews.llvm.org/D38113#878852, @hfinkel wrote: > In https://reviews.llvm.org/D38113#877874, @Anastasia wrote: > > > The problem of adding this attribute conservatively for all functions is > > that it prevents some optimizations to happen. I agree to commit

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-25 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. Do we need an option to disable this? In case it causes regression in some applications and users want to disable it. At least for debugging. Comment at: test/CodeGenOpenCL/convergent.cl:73 // CHECK: %[[tobool_pr:.+]] = phi i1 [ true, %[[if_then]] ],

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-22 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D38113#877874, @Anastasia wrote: > The problem of adding this attribute conservatively for all functions is that > it prevents some optimizations to happen. I agree to commit this as a > temporary fix to guarantee correctness of generated cod

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-22 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment. > Yes, that's why if it would be responsibility of the kernel developer to > specify this explicitly we could avoid this complications in the compiler. > But if we add it into the language now we still need to support the > correctness for the code written with the earli

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-22 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments. Comment at: test/CodeGenOpenCL/convergent.cl:130 +// CHECK: attributes #0 = { noinline norecurse nounwind " +// CHECK: attributes #1 = { {{[^}]*}}convergent{{[^}]*}} } +// CHECK: attributes #2 = { {{[^}]*}}convergent{{[^}]*}} } We

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-22 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. In https://reviews.llvm.org/D38113#877906, @jlebar wrote: > > The problem of adding this attribute conservatively for all functions is > > that it prevents some optimizations to happen. > > function-attrs removes the convergent attribute from anything it can prove > d

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-21 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment. > The problem of adding this attribute conservatively for all functions is that > it prevents some optimizations to happen. function-attrs removes the convergent attribute from anything it can prove does not call a convergent function. I agree this is a nonoptimal solut

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-21 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment. The problem of adding this attribute conservatively for all functions is that it prevents some optimizations to happen. I agree to commit this as a temporary fix to guarantee correctness of generated code. But if we ask to add the `convergent` attribute into the spec

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-20 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 116125. arsenm added a comment. Herald added a subscriber: nhaehnle. Missed test update https://reviews.llvm.org/D38113 Files: include/clang/Basic/LangOptions.h lib/CodeGen/CGCall.cpp test/CodeGenOpenCL/amdgpu-attrs.cl test/CodeGenOpenCL/convergent.c

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-20 Thread Justin Lebar via Phabricator via cfe-commits
jlebar added a comment. LGTM for the changes other than the test (I don't read opencl). https://reviews.llvm.org/D38113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38113: OpenCL: Assume functions are convergent

2017-09-20 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm created this revision. Herald added a subscriber: wdng. This was done for CUDA functions in r261779, and for the same reason this also needs to be done for OpenCL. An arbitrary function could have a barrier() call in it, which in turn requires the calling function to be convergent. https: