[PATCH] D65141: [DivergenceAnalysis] Add methods for querying divergence at use

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 24 00:23:01 PDT 2019


nhaehnle added a comment.

Nice work! It's missing tests though -- I'm not sure if we want to print this additional information in the divergence printer, but definitely a test for the atomic optimizer pass is required.



================
Comment at: llvm/lib/Analysis/LegacyDivergenceAnalysis.cpp:204-206
     for (auto &I : *InfluencedBB)
-      findUsersOutsideInfluenceRegion(I, InfluenceRegion);
+      if (!DV.count(&I))
+        findUsersOutsideInfluenceRegion(I, InfluenceRegion);
----------------
Please put braces around the `for` body (multi-line body).


================
Comment at: llvm/lib/Analysis/LegacyDivergenceAnalysis.cpp:365
+  }
+  return DivergentValues.count(U->getUser()) || DivergentUses.count(U);
+}
----------------
What's the rationale for checking the user here as well?

For example, if the user is a binary operation that is divergent only because the **other** use is divergent, then I'd expect this function to return false...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65141





More information about the llvm-commits mailing list