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

It’s not about weird restrictions we are adding, it’s about the fact that for
that specific address space we cannot dynamically index into the memory.

The address needs to be a constant of some sort in the selection (it’s an
hardware limitation, there’s nothing we can do about it) and the fact that it is
transformed into a PHI makes it not constant anymore.

We will need to undo this into Codegen prepare by either reverse that putting
the load back into the predecessors and creating a PHI of the result of the
loads.

Having a way to opt-out or control what you want to sink I still think would be
a added value to the optimization.

Marcello 
> On 2 Sep 2016, at 15:05, James Molloy <James.Molloy at arm.com>
wrote:
> 
> Hi,
> 
>> On 2 Sep 2016, at 22:40, Marcello Maggioni <mmaggioni at apple.com
<mailto:mmaggioni at apple.com>> wrote:
>> 
>>> 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.
> 
> Are you referring to a downstream intrinsic, an upstream intrinsic or are
you somehow adding strange restrictions yourself on what a load instruction can
and can’t do?
> 
> There’s a function canReplaceOperandWithVariable() that this optimisation
uses to decide if it’s possible to do this. We use this to avoid making things
that must be constant (like your example) variable. Perhaps this is doing
something wrong? If you’re adding arbitrary restrictions on what loads can do
though that’s never going to go well - I’m sorry but we just don’t support that
use case. That’s what intrinsics are for.
> 
> > Just a question. Why implementing it in SimplifyCFG and not as a
separate pass like JumpThreading or something like that?
> 
> Because sinking instructions into successors already exists in SimplifyCFG
and has done for years. This is a small modification to make it a bit more
clever. SimplifyCFG does lots of this kind of stuff already (switch->lookup
table for example).
> 
> Cheers,
> 
> James
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160902/7516e99b/attachment.html>

On 9/2/2016 6:54 PM, Marcello Maggioni via llvm-dev
wrote:
> It’s not about weird restrictions we are adding, it’s about the fact
> that for that specific address space we cannot dynamically index into
> the memory.
>
> The address needs to be a constant of some sort in the selection (it’s
> an hardware limitation, there’s nothing we can do about it) and the fact
> that it is transformed into a PHI makes it not constant anymore.
Could you use an intrinsic for the load?

An optimization could reasonably be expected not to replace a constant 
input to an intrinsic with a variable, so even if it fails at the 
moment, it could be addressed.

-Krzysztof

Hi,

Yes, indeed, if you changed these loads into intrinsic loads this problem would
go away, because we don’t transform intrinsics for this *exact reason*. There
are other intrinsics that must take a constant operand - I’ve been working on a
way to determine accurately if this is the case or not but it’s not in mainline
yet.

I think having this restriction on the IR is outside LLVM’s model, and it’s
likely break elsewhere too. I understand the hardware restriction, I just feel
that it could be modelled more accurately in the IR :)

Cheers,

James
> On 3 Sep 2016, at 01:06, Krzysztof Parzyszek via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>
> On 9/2/2016 6:54 PM, Marcello Maggioni via llvm-dev wrote:
>> It’s not about weird restrictions we are adding, it’s about the fact
>> that for that specific address space we cannot dynamically index into
>> the memory.
>>
>> The address needs to be a constant of some sort in the selection (it’s
>> an hardware limitation, there’s nothing we can do about it) and the
fact
>> that it is transformed into a PHI makes it not constant anymore.
>
> Could you use an intrinsic for the load?
>
> An optimization could reasonably be expected not to replace a constant
input to an intrinsic with a variable, so even if it fails at the moment, it
could be addressed.
>
> -Krzysztof
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
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.