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

Reply via email to