[PATCH] D79037: [StructurizeCFG] Fix region nodes ordering

Sat May 2 05:48:01 PDT 2020

ekatz added a comment.

In D79037#2016160 <https://reviews.llvm.org/D79037#2016160>, @sameerds wrote:

> It seems the new commit is an incremental update on the previous commit. Can you please submit a single squashed commit, so that the change can be compared against the original state of the trunk?

I am not sure what do you mean. In this page in the Phabricator, under [Revision Contents]->[History] you can choose which of the patch revisions to diff. The default is "Base" against the latest "Diff no.".

> Also, the following tests were created specifically to demonstrate the wrong ordering:
> 
> - test/Transforms/StructurizeCFG/workarounds/needs-unified-loop-exits.ll
> - test/Transforms/StructurizeCFG/workarounds/needs-fr-ule.ll

Great! Didn't know about those. I'll update them, and if everything is OK, I'll move them out of the "workarounds" directory.

> If the proposed change is correct, then the example in these tests will be structurized correctly even after removing the "-unify-loop-exits" argument. These tests should also be checked and then updated accordingly.
> 
> In fact, it would be nice to update the following tests. But they have a very confusing history and they are specific to AMDGPU. I can volunteer to do this later as a follow up.
> 
> - test/Transforms/StructurizeCFG/AMDGPU/backedge-id-bug.ll
> - test/Transforms/StructurizeCFG/AMDGPU/backedge-id-bug-xfail.ll

I'll try to take a look at those as well. These are "XFAIL" right now. If I'm understanding correctly, You assume they should pass with this patch?

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

https://reviews.llvm.org/D79037