[PATCH] D85818: [UnifyFunctionExitNodes] Fix Modified status for unreachable blocks

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp:116
+  Changed |= unifyReturnBlocks(F);
   return true;
 }
----------------
Assuming intention was to do `return Changed;` here.


================
Comment at: llvm/test/Transforms/UnifyFunctionExitNodes/unreachable-blocks-status.ll:3
+
+; The pass did previously not report the correct Modified status in the case
+; where a function had at most one return block, and an unified unreachable
----------------
I guess one would have neeed to enable EXPENSIVE_CHECKS for this test case to fail in the past (but only since the check was addded to EXPENSIVE_CHECKS). Could it be worth the trouble to use -debug-pass=Details and check that the pass returns that it has modified both @foo and @bar? In some sense getting rid of the dependency to EXPENSIVE_CHECKS.

On the other hand, I'm not sure if -debug-pass=Details works with new-pm. But mergereturn has not been ported to the new-pm either, so I'm not sure about the future plans for the pass. Maybe we shouldn't spend too much time here. So I'm happy either way.
 


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

https://reviews.llvm.org/D85818