[PATCH] D43743: StructurizeCFG: Test for branch divergence correctly

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Transforms/Scalar/StructurizeCFG.cpp:937-938
     // TODO: We could probably be smarter here with how we handle sub-regions.
-    auto &DA = getAnalysis<DivergenceAnalysis>();
-    if (hasOnlyUniformBranches(R, DA)) {
+    unsigned UniformMDKindID =
+        R->getEntry()->getContext().getMDKindID("structurizecfg.uniform");
+    DA = &getAnalysis<DivergenceAnalysis>();
----------------
nhaehnle wrote:
> arsenm wrote:
> > arsenm wrote:
> > > This seems to be increasing the reliance on metadata? Is this passing information from one RegionPass invocation to another? Metadata isn't really intended to be way to pass information like that
> > I have a patch to stop using a RegionPass and instead just use the RegionInfo directly in a FunctionPass. Would that help avoid this?
> I think the metadata issue here is minimal, because we're only looking for metadata that this pass adds itself. But yes, moving towards a FunctionPass would certainly help clean this up.
I think this at least needs a comment explicitly mentioning how this is passing info between pass invocations and needs to be fixed


Repository:
  rL LLVM

https://reviews.llvm.org/D43743