On 11/17/20 9:46 AM, Jakub Jelinek wrote:
> On Tue, Nov 17, 2020 at 05:29:57PM +0100, Philipp Tomsich wrote:
>>>> In other words, the change to VRP canonicalizes what a lshift_expr with an
>>>> shift-amount outside of the type width means... it doesn't assume anything
>>>> about the original language.
>>>> Do we assume that a LSHIFT_EXPR has the same semantics as for a
>>>> C-language shift-left? If so, then pre should not generate the LSHIFT_EXPR
>>>> for _9... or we might even catch this later in path isolation (as
>>>> undefined
>>>> behavior, insert a __builtin_trap() and emit a warning)?
>>>>
>>>> Note that in his comment to patch 2/2, Jim has noted that user code for
>>>> RISC-V may assume a truncation of the shift-operand...
>>> What I'd suggest doing would be to leave the invalid shift count in the
>>> IL in VRP, then extend the erroneous path isolation code to turn an
>>> invalid shift into a trap (conditionally of course).
>> You had originally suggested to add this to VRP ...
>> Given the various comments to this patch, do you still want any of
>> this in VRP or
>> would you rather see this only in path isolation?
> Well, I said if we want to do it at all, it should be done in VRP, because
> there is not really a difference between ((int) x) << 32 and ((int) x) << y
> for y in [32, 137] etc.
Right. VRP is the right place to discover, but I'm not sure it's the
right place to clean up the mess though.
> Otherwise it is the general question of what to do upon proven UB, and that
> is a topic discussed several years during Cauldron that it would be nice to
> have switches where users can choose what to do in that case,
> __builtin_unreachable (), __builtin_trap (), ... and another thing is where
> we should warn about it (tight e.g. to the __builtin_warning thing, because
> we don't want these warnings for dead code).
>
> So, e.g. if we had __builtin_warning (dunno where Martin S. is with that),
> we could e.g. queue a __builtin_warning and add __builtin_unreachable (or
> other possibilities), or e.g. during VRP just canonicalize proven always
> out of bound shifts to shifts by an out of bound constant and let some later
> pass warn and/or add __builtin_warning.
So the idea is to start funneling this through the path isolation code
and handle the various strategies there.
__builtin_warning is on hold pending a rework to make it act more like a
debug statement than a builtin function call. The latter impacts
various heuristics, which would mean that it could impact codegen which
would highly undesirable.
jeff
>
> Jakub
