[PATCH] D81788: [OpenMPOpt] ICV Tracking

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Some nits to be addressed, otherwise LGTM.

Address the clang-tidy warnings where possible please.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:799
+
+    for (InternalControlVar ICV : TrackableICVs)
+      if (deduplicateICVGetters(ICV, A))
----------------
`&` and below. I assume you don't want to copy it all the time.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:830
+    return Changed;
+  }
+
----------------
Note: Once we do proper optimistic initialization we need to compute the replacement value in the update. For now this is fine.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:864
+    }
+  };
+
----------------
Nit: If you declare this as `template<> struct DenseMapInfo<ICVValue>` you don't need to specify it with the map later.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:911
+                             Attributor &A) override {
+    BasicBlock *CurrBB = const_cast<BasicBlock *>(I->getParent());
+
----------------
Wouldn't a const block do just fine below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81788