llvm dev - Jun 2019 - Planned change to IR semantics: constant expressions never have undefined behavior

On Fri, Jun 14, 2019 at 7:10 PM Chris Lattner <clattner at nondot.org>
wrote:
>
>
> On Jun 14, 2019, at 4:06 PM, Cameron McInally <cameron.mcinally at
nyu.edu>
> wrote:
>
>
>
> On Fri, Jun 14, 2019 at 6:58 PM Chris Lattner via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>>
>>
>> On Jun 14, 2019, at 3:24 PM, Eli Friedman via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>> See https://reviews.llvm.org/D63036
>>
<https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D63036&d=DwMFaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=O_4M49EtSpZ_-BQYeigzGv0P4__noMcSu2RYEjS1vKs&m=HPH8LbjOYmP3G9p6HJYKLtOFGQQJ0aWeEc7QU46PkOw&s=Q-PK-STEJTkFgcTQeOjIR7RY4e_iIHz191Qk_8syQlA&e=>
>>  for the full patch and description. Essentially, the Constant::canTrap
>> API is going away.  To make this work, the semantics of division and
>> remainder constant expressions are changing slightly, so division by
zero
>> produces poison, not undefined behavior.  (The corresponding
instructions
>> still have undefined behavior.)  This change should make writing and
>> understanding IR optimizations easier.
>>
>>
>> Is anyone interested and willing to make a more profound change to
llvm’s
>> constants?  We should really remove all the trapping operators.  The
only
>> reason they exist in the first place is to allow use of Constant::get()
as
>> a high level constant folding API.  We could replace that, eliminate
the
>> trapping operators, and thus eliminate a ton of complexity and bugs…
>>
>
> Hi Chris,
>
> If I'm understanding your proposal correctly, the constrained FP
intrinsic
> work could make use of that code -- in the future. Eventually we'll
want to
> optimize the constrained FP intrinsics and being able to constant fold
> non-trapping instructions is important.
>
>
> Right: in terms of primary constant folding entry points, the
> ConstExpr::get sort of methods have always been a mistake (and I’ve never
> gotten around to fixing them).
>
Ok, I think I understand now. So we'd retain the ability to constant fold
operations that may trap, possibly returning trap flags, through APFloat.
If that's correct, that would work for the constrained FP intrinsics
project. No big deal.

> A better thing to do is to make ConstExpr’s have their own enum of
> “operations”, and make them correspond to the things that show up in a
> global variable initializer (e.g. relocatable expressions). There is no
> reason for constant expressions to mirror the instruction enum, and there
> is lots of harm caused by it.
>
You may have lost me here, but that's ok. If you're suggesting not
switching on the Instruction OpCodes in ConstantExpr::get(...) et al, to
encapsulate the ConstantExpr class, then +1 from me.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190614/7a9f837e/attachment.html>

From: Cameron McInally <cameron.mcinally at nyu.edu>
Sent: Friday, June 14, 2019 5:40 PM
To: Chris Lattner <clattner at nondot.org>
Cc: Eli Friedman <efriedma at quicinc.com>; llvm-dev <llvm-dev at
lists.llvm.org>
Subject: [EXT] Re: [llvm-dev] Planned change to IR semantics: constant
expressions never have undefined behavior

On Fri, Jun 14, 2019 at 7:10 PM Chris Lattner <clattner at
nondot.org<mailto:clattner at nondot.org>> wrote:



On Jun 14, 2019, at 4:06 PM, Cameron McInally <cameron.mcinally at
nyu.edu<mailto:cameron.mcinally at nyu.edu>> wrote:



On Fri, Jun 14, 2019 at 6:58 PM Chris Lattner via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:



On Jun 14, 2019, at 3:24 PM, Eli Friedman via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

See
https://reviews.llvm.org/D63036<https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D63036&d=DwMFaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=O_4M49EtSpZ_-BQYeigzGv0P4__noMcSu2RYEjS1vKs&m=HPH8LbjOYmP3G9p6HJYKLtOFGQQJ0aWeEc7QU46PkOw&s=Q-PK-STEJTkFgcTQeOjIR7RY4e_iIHz191Qk_8syQlA&e=>
for the full patch and description. Essentially, the Constant::canTrap API is
going away.  To make this work, the semantics of division and remainder constant
expressions are changing slightly, so division by zero produces poison, not
undefined behavior.  (The corresponding instructions still have undefined
behavior.)  This change should make writing and understanding IR optimizations
easier.

Is anyone interested and willing to make a more profound change to llvm’s
constants?  We should really remove all the trapping operators.  The only reason
they exist in the first place is to allow use of Constant::get() as a high level
constant folding API.  We could replace that, eliminate the trapping operators,
and thus eliminate a ton of complexity and bugs…

Hi Chris,

If I'm understanding your proposal correctly, the constrained FP intrinsic
work could make use of that code -- in the future. Eventually we'll want to
optimize the constrained FP intrinsics and being able to constant fold
non-trapping instructions is important.

Right: in terms of primary constant folding entry points, the ConstExpr::get
sort of methods have always been a mistake (and I’ve never gotten around to
fixing them).

Ok, I think I understand now. So we'd retain the ability to constant fold
operations that may trap, possibly returning trap flags, through APFloat. If
that's correct, that would work for the constrained FP intrinsics project.
No big deal.

Yes, this would be specifically about the entry points that produce a
ConstantExpr.

A better thing to do is to make ConstExpr’s have their own enum of “operations”,
and make them correspond to the things that show up in a global variable
initializer (e.g. relocatable expressions). There is no reason for constant
expressions to mirror the instruction enum, and there is lots of harm caused by
it.

You may have lost me here, but that's ok. If you're suggesting not
switching on the Instruction OpCodes in ConstantExpr::get(...) et al, to
encapsulate the ConstantExpr class, then +1 from me.

The idea here is actually to eliminate the underlying expressions themselves, so
it would no longer be legal to write “sdiv (i32 […], i32 […])” as a constant
expression at all in IR.

The hardest part of this is probably figuring out how to handle AutoUpgrade;
there aren’t that many transforms which actually try to create an sdiv constant
expression, and it shouldn’t be too hard to make them fail gracefully.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190615/429b7f71/attachment.html>

On Jun 14, 2019, at 6:41 PM, Eli Friedman <efriedma at quicinc.com>
wrote:
> 
> A better thing to do is to make ConstExpr’s have their own enum of
“operations”, and make them correspond to the things that show up in a global
variable initializer (e.g. relocatable expressions). There is no reason for
constant expressions to mirror the instruction enum, and there is lots of harm
caused by it.
>  
> You may have lost me here, but that's ok. If you're suggesting not
switching on the Instruction OpCodes in ConstantExpr::get(...) et al, to
encapsulate the ConstantExpr class, then +1 from me.
>  
> The idea here is actually to eliminate the underlying expressions
themselves, so it would no longer be legal to write “sdiv (i32 […], i32 […])” as
a constant expression at all in IR.
>  
> The hardest part of this is probably figuring out how to handle
AutoUpgrade;

Exactly.  I haven’t actually tried this, but in my imagination, it isn’t too
bad: there are three cases for a constant expr containing a div:

1) constant used by normal instruction: expand inline into the current block,
and use the non constant value.
2) phi node: expand into the appropriate predecessor.
3) Global variable init or other thing: error, not supported by a backend
anyway.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190614/be3c79e1/attachment-0001.html>