[PATCH] D22630: Loop rotation

Bjorn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 31 09:46:31 PDT 2016


bjope added inline comments.


================
Comment at: llvm/test/Transforms/LoopRotate/phi-duplicate.ll:31
 
 ; Should only end up with one phi.
 ; CHECK-LABEL:      define void @test(
----------------
The sole purpose with this test case seems to be to verify PR5837, and to make sure that there is only one phi (no duplicates created). Now you have introduced new CHECK statements to verify that we do get multple phi nodes.

If we still think that this test case is important and that that we only want want phi for this code, then I guess it still should verify that there is only one phi.
If the world has changed and we now think that multiple phi nodes are wonderful, then I either expect that the test case is obsolete and can be removed. Or there should be a pretty good informative explanation why the test case has mutated in this way (no longer verifying the same thing as it initially was intended do verify).


https://reviews.llvm.org/D22630





More information about the llvm-commits mailing list