Jeff, On Tue, 17 Nov 2020 at 00:38, Jeff Law <l...@redhat.com> wrote:
> > On 11/16/20 11:57 AM, Philipp Tomsich wrote: > > From: Philipp Tomsich <p...@gnu.org> > > > > While most shifts wider than the bitwidth of a type will be caught by > > other passes, it is possible that these show up for VRP. > > Consider the following example: > > int func (int a, int b, int c) > > { > > return (a << ((b && c) - 1)); > > } > > > > This adds simplify_using_ranges::simplify_lshift_using_ranges to > > detect and rewrite such cases. If the intersection of meaningful > > shift amounts for the underlying type and the value-range computed > > for the shift-amount (whether an integer constant or a variable) is > > empty, the statement is replaced with the zero-constant of the same > > precision as the result. > > > > gcc/ChangeLog: > > > > * vr-values.h (simplify_using_ranges): Declare. > > * vr-values.c (simplify_lshift_using_ranges): New function. > > (simplify): Use simplify_lshift_using_ranges for LSHIFT_EXPR. > > Umm, isn't this a shift wider than the bitwidth undefined behavior? We > should be generating warnings for that, not trying to further optimize > it :-) > The shift is undefined behavior on the language level (for C) and a warning will be generated, if such a shift is encountered; additionally, the shift will be replaced with the value 0. However, in the above case, the shift is generated only in the middle end: At 136t.walloca, I still have: > # RANGE [-1, 0] > _1 = iftmp.1_2 + -1; > _6 = a_5(D) << _1; Whereas at 137t.pre, this is changed into: > Found partial redundancy for expression {lshift_expr,a_5(D),_1} (0006) > Inserted _9 = a_5(D) << -1; 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... Philipp.