On Wed, Nov 26, 2014 at 06:50:29PM +0100, Jakub Jelinek wrote: > On Wed, Nov 26, 2014 at 06:39:44PM +0100, Marek Polacek wrote: > > Both C11 and C++14 standards specify that integral promotions are > > performed on both operands of a shift-expression. This we do just > > fine. But then we convert the right operand to integer_type_node. > > Not only is this unnecessary, it can also be harfmul, because for > > e.g. > > void > > foo (unsigned int x) > > { > > if (-1 >> x != -1) > > bar (); > > } > > with (int) x we lose info that x is nonnegative, which means that > > tree_expr_nonnegative_p cannot fold this expr. Another problem > > with the conversion is that we weren't able to detect e.g. shift > > by 0x100000000ULL, since after the conversion this is 0. > > Have you checked what the expander does with it? Say trying to > shift something by __int128 count or similar might upset it.
I tried int main () { __int128 x = 1; __int128 y = 200; __int128 z; asm ("" : "+g" (x), "+g" (y)); z = x << y; asm ("" : "+g" (z)); return z; } and that works (even with ubsan) as expected. I haven't checked .expand (I don't think I'd be able to judge the difference anyway), but on the testcase above the assembly is the same with -O, with -O0 I see: movq %rbx, -24(%rbp) movq %rax, -48(%rbp) movq %rdx, -40(%rbp) - movq -48(%rbp), %rax - movl %eax, %ecx movq -32(%rbp), %rax movq -24(%rbp), %rdx + movzbl -48(%rbp), %ecx shldq %rax, %rdx salq %cl, %rax testb $64, %cl > Wonder about if we don't make similar assumptions in the middle-end. > It might make a difference (sometimes positive, sometimes negative) > for vectorization too. I don't really know; I assume the testsuite would have catched it if something went awry. > > 2014-11-26 Marek Polacek <pola...@redhat.com> > > > > PR c/63862 > > c-family/ > > * c-ubsan.c (ubsan_instrument_shift): Change the type of a MINUS_EXPR > > to op1_utype. > > This part is ok regardless of the rest. Thanks, Marek