[llvm] [OpenMPOpt] Fix incorrect end-of-kernel barrier removal (PR #65670)
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 8 13:10:13 PDT 2023
================
@@ -2676,17 +2676,19 @@ struct AAExecutionDomainFunction : public AAExecutionDomain {
if (!ED.EncounteredAssumes.empty() && !A.isModulePass())
return;
- // We can remove this barrier, if it is one, or all aligned barriers
- // reaching the kernel end. In the latter case we can transitively work
- // our way back until we find a barrier that guards a side-effect if we
- // are dealing with the kernel end here.
+ // We can remove this barrier, if it is one, or aligned barriers reaching
+ // the kernel end (if CB is nullptr). Aligned barriers reaching the kernel
+ // end may have other successors besides the kernel end (especially if
+ // they're in loops) with non-local side-effects, so those barriers can
+ // only be removed if they also only reach the kernel end. If those
+ // barriers have other barriers reaching them, those can be transitively
+ // removed as well.
if (CB) {
DeletedBarriers.insert(CB);
A.deleteAfterManifest(*CB);
++NumBarriersEliminated;
Changed = ChangeStatus::CHANGED;
} else if (!ED.AlignedBarriers.empty()) {
----------------
jdoerfert wrote:
We might want to treat the kernel end special, but it is not an aligned barrier. Threads are allowed to run into the kernel end independently, and the others can do something else in the meantime, e.g., wait in an aligned barrier. What I'm trying to say is that our current abstraction is too coarse-grained. That said, you might be right that for the purpose of barrier elimination, we can continue to "handle" the kernel end.
That said, I'm still not sure if this is the right way to fix this. I think the change in multiple_blocks_functions_kernel_effects_0 actually exposes a different error we should first address.
https://github.com/llvm/llvm-project/pull/65670
More information about the llvm-commits
mailing list