[PATCH] D140463: [OpenMP] Merge barrier elimination into AAExecutionDomain

[Shilei Tian via Phabricator via llvm-commits]

tianshilei1992 added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:3334
 
+  /// Helper function to determine if \p is an aligned (GPU) barrier.
+  /// Aligned barriers have to be executed by all threads.
----------------



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:5007-5010
+    ExecutionDomainTy() {}
+    ~ExecutionDomainTy() {
+      // Cleanup has to happen by the user.
+    }
----------------
No need for the two functions.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:5040
+    BarriersSetTy *AlignedBarriers = nullptr;
+    AssumesSetTy *EncounteredAssumes = nullptr;
+  };
----------------
It is not necessary to make it a pointer, same as `AlignedBarriers`.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2553
 
+  ~AAExecutionDomainFunction() {
+    for (auto &It : BEDMap) {
----------------
The whole function can be avoided if they are not pointer. Wasting tiny piece of memory (if they are empty) is not a big deal.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2617
+        Changed = ChangeStatus::CHANGED;
+      } else if (ED.AlignedBarriers) {
+        NumBarriersEliminated += ED.AlignedBarriers->size();
----------------
And this could be `!ED.AlignedBarriers.empty()`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140463/new/

https://reviews.llvm.org/D140463