[llvm] [SPIR-V] Fix block sorting with irreducible CFG (PR #116996)

[Nathan Gauër via llvm-commits]

Keenuts wrote:

> > What's important in such case is that loops belongs before the exit node.
> 
> Why is this important? I thought that was part of the spec, but I cannot find it. Is it part of how the structurizer is implemented?

For the problem of BB sorting for binary output, I don't think there is a spec bit that says so.
It's really just a matter of how the structurizer works.

We have 3 parts:
 - the structurizer, which relies on the partial ordering visitor, and need the loop bit.
 - the partial ordering, which gives the node rank, and loop bit.
 - and the sorting, which only requires dominance ordering.

Because I had the partial ordering, which provided a super-set of the requirements for sorting, I used it to sort blocks.
But indeed we could split the two: have a sorting function which only relies on dominance pre-order, and the partial ordering, which is required for the structurizer.


> For changes to this PR, you should update the comments in SPIRVUtils.h to mention the case where it might fail to give a topological order

Sure, I can add a comment. But for irreducible, this order is not defined, so it does not "fail". It just gives 1 version of it.

> **EDIT:** Now that I think about it. I don't like that a class could silently fail to do what it is suppose to do. It could be confusing for people who try to us it in a different context.

Once again, IMO it doesn't fail. Topological sorting is not defined the graphs with cycles, even less for irreducible graphs. Hence this is a kinda-topological with a bias on the loop body ordering.


So I see 2 ways forward:
 - keep this as-is, using the kinda-topological-sorting to sort blocks.
 - reimplement the sorting function to only use pre-order so we stick to the spec requirements, and leave the visitor for the structurizer.



https://github.com/llvm/llvm-project/pull/116996