[PATCH] D24805: [GVNSink] Initial GVNSink prototype

[James Molloy via llvm-commits]

Hi,

Thanks for the quick reply; this is a very helpful and constructive discussion :)

The oversimplification and repetition below is for my own understanding and so that I can better understand your replies, as you know this subject far better than I.

A: store 1
B: store 2
C: store 3
D:

I understand that in a classical sinking algorithm we'd want to walk backwards:
  1) Sink 'store 3' from C to D
  2) Sink 'store 2' from B to D
  3) Sink 'store 1' from A to D

A key to this working is that each instruction, when considered, was sunk as far as it could be. If we had added the restriction that an instruction could only be sunk to its immediate successor block, that algorithm would have taken three iterations to reach a fixpoint.

GVNSink can (at the moment at least) only sink instructions from a block into its immediate successor. This is what allows it to model accurately enough how many PHIs it is going to create and whether sinking is actually worthwhile (and how much sinking). I don't believe it would be able to handle all the cases it currently does if it didn't have this restriction (but would be able to handle other cases that it currently can't, so I see that very much as an enhancement I'd like to add later).

With that restriction, traversing over the CFG in RPO and considering instructions in immediate predecessors in reverse order seems reasonable. On the above CFG:

  1) Sink 'store 1' from A to B
  2) Sink 'store 2' from B to C
  3) Sink 'store 1' from B to C
  4) Sink 'store 3' from C to D
  5) Sink 'store 2' from C to D
  6) Sink 'store 1' from C to D

It is more work (it reconsiders nodes), but does only require a single pass and reconsideration is quite quick as most of the calculations have already been done.

I just did a simple check by running another iteration of GVNSink (simulating a second iteration) and asserting if the second iteration sunk anything; this made it all the way through the test-suite/spec/whatever without asserting, so I do believe what I have right now hits the interesting cases in one pass.

Cheers,

James


> On 22 May 2017, at 15:46, Daniel Berlin via Phabricator <reviews at reviews.llvm.org> wrote:
>
> dberlin added a comment.
>
> In https://reviews.llvm.org/D24805#760949, @jmolloy wrote:
>
>> Hi Danny,
>>
>> Thanks for making me think more about the iteration order. Regardless of the difficulties of forming inverse RPO, I now think I was completely wrong about the preferred iteration order.
>
>
> I don't believe you are.
>
> Most sinking passes operate in IRPO or PDT order.
>
>> For a sinking pass, surely a *forward* walk (preds before succs) is more optimal than a backward walk.
>
> Nope.
>
> For most sinking passes, the blocker is the thing below the current instructions
>
>> Pushing instructions down the graph exposes more opportunities for sinking later on.
>
> Yes, which is why things operate in reverse-local, succs before preds order.
>
> given
>
>  A:
>  store 1
>  B:
>  store 2
>  C
>  store 3
>
> They want to do "sink store 3, sink store 2, sink store 1".
>
> This is IRPO order, walking each basic block backwards.
>
> If you do sink store 1, sink store 2, sink store 3, you may only be able to sink store 1 to store 2's location, and store 2 to store 3's location.
> Then once you move store 3, you must repeat.
>
> The above may take 3 iterations to be optimal in a forward RPO pass
> It takes 1 in a IRPO pass.
>
>> So I think a single pass, RPO walk of the CFG is sufficient (and indeed passes all my testcases and all the internal testing I can throw at it).
>
> This is pretty surprising.
>
> Given two must-aliased stores, for example, i can't see how this would sink them as far as possible in one iteration.
>
>
> Repository:
>  rL LLVM
>
> https://reviews.llvm.org/D24805
>
>
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.