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.
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.
Jakub
