[llvm-dev] Problem with "[SimplifyCFG] Handle tail-sinking of more than 2 incoming branches"

[Marcello Maggioni via llvm-dev]

Probably the issue is solvable in some Codegen prepare pass.

That said I still believe some kind of control on if we would like to implement this or not could be useful.

Just a question. Why implementing it in SimplifyCFG and not as a separate pass like JumpThreading or something like that? The transformation itself doesn’t seem to fit much in SimplifyCFG.
> On 2 Sep 2016, at 13:35, Michael Kuperstein <mkuper at google.com> wrote:
> 
> Hello Marcello,
> 
> We're currently planning to disable this optimization - at least temporarily - for cases where we'll end up creating PHIs of pointers for performance reasons (see PR30188).
> 
> Having said that - I don't believe this is something you can rely on for correctness.
> From an IR semantics perspective, this is a completely legal transformation that can be introduced by any optimization pass, for whatever reason. We can't control that throughout the entire optimization stack with a TTI hook. Two potential solutions I can see are to either (a) teach your backend to handle it (by disambiguating these loads in a pre-ISel IR pass), or (b) use some kind of type-system trick to make it impossible (say, assign a different address space to each distinct pointer in the front end).
> 
> Thanks,
>   Michael
> 
> On Fri, Sep 2, 2016 at 11:39 AM, Marcello Maggioni via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> Hello,
> 
> I’m getting some failures on our internal testing happening after this commit.
> 
> I dug a little bit into it and it ended up begin caused by the fact that this optimization is sinking some instructions that shouldn’t be sunk
> 
> In particular we have some loads that need to have a constant address to be selected, because they load from a “special address space”.
> 
> What the optimization is doing is extracting the “getelementptr” , making an instruction out of it, sinking the load and using a PHI to select between the address.
> This breaks our selection of this types of loads.
> 
> We need a way to tell this code to not do that.
> 
> The solutions I kind of see for this is:
> 
> - Add some TTI hook that stops this sinking from happening for instructions that don’t support that like in this case (maybe added in canSinkInstructions())
> 
> - Add some TTI hook that is a little bit coarser that stops the optimization to run altogether. In particular this thing is creating “unstructured control-flow” in some cases and this is not always good (and actually generally bad) on GPU targets.
> 
> - Put the optimization in another separate pass. It seems to be doing stuff that is a little bit more than just “Simplifying the CFG” actually. It is actually making it more complex by splitting blocks around. I wonder if it would make sense for it to live it in a separate pass that can be enabled or disabled at the compiler pipeline level so that one that wants to opt out can just avoid to add the pass to the pipeline.
> 
> Open for suggestions on what the best solution for this is.
> 
> Thanks,
> Marcello
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160902/88267cce/attachment.html>