[PATCH] D37575: [BasicBlock] add new function removeEdge()

Brian Rzycki via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 27 09:12:31 PDT 2017


brzycki added a comment.

In https://reviews.llvm.org/D37575#882225, @dberlin wrote:

> (repeating comments i forgot to add to review) 
>  This definitely needs an RFC.


Thanks for the feedback Daniel. I sent the RFC email to llvm-dev this morning.

> I'll note your approach is badly O(N) in some cases.
>  You could try using the uses instead (they know what operand number and their uses).
>  I suspect that's hard to pass down everywhere in each pass.

I agree several cases are O(N) (switch, catchswitch, indirect branch). I was not aware uses contained the operand number and I do agree that would save time (and probably simplify some of the logic). One of the reasons I wrote the code this way was investigating actual pass code that changed terminator instructions. A good deal of the original idea came from the huge if()..else blocks inside Local.cpp routines. I also haven't examined the code enough to determine if requiring uses would act as a barrier to use.

> In general If we are going to go this route, either we should: have a real CFG structure that isn't going to be O(N) to remove an edge.

I definitely agree that a real CFG structure is needed. Thinking about a higher-level change to the shape of the IR and then switching to the intricacies of the IR to implement it can be quite frustrating.

> or incrementally, cache data in basic blocks (successor, predecessor lists and uses) that gets invalidated, so you can find the removal points in constant time.
>  The latter is likely to require a lot of verification to not generate hard to track down bugs in the compiler.

It would be nice to simply query a method of a BB without having to to write for (Successor: Successors())....Validating the cache would be non-trivial and I would consider it a failure if the user had to worry about the internal state (and call routines to verify/fix it at the call site).

My hope is we can start moving in a meaningful direction to reduce this problem without getting overwhelmed with the changes needed to be added. I greatly prefer incremental changes to an existing design as a method to evolve the code.


https://reviews.llvm.org/D37575





More information about the llvm-commits mailing list