Re: [Patch] sext_hwi: Avoid left shift of negative value undefined behaviour

[Mikael Morin]

Le 12/08/2015 13:09, Richard Biener a écrit :
On Wed, Aug 12, 2015 at 1:07 PM, Markus Trippelsdorf
<mar...@trippelsdorf.de> wrote:
On 2015.08.12 at 13:01 +0200, Richard Biener wrote:
What kind of error does ubsan run into?  That is, for which 'prec'?

See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67042

Ugh.  Stupid C.

My fear is that with so many stmts sext_hwi isn't any longer considered for
inlining, even if prec is constant.  What's the generated code for its
out-of line
copy with/without the patch?


The difference on x86_64 is:

        .cfi_startproc
        cmpl    $64, %esi
        je      .L3
-       movl    $64, %ecx
-       subl    %esi, %ecx
-       salq    %cl, %rdi
+       leal    -1(%rsi), %ecx
+       movl    $1, %eax
+       movq    %rax, %rdx
+       salq    %cl, %rdx
+       movl    %esi, %ecx
+       salq    %cl, %rax
+       subq    $1, %rax
+       andq    %rax, %rdi
+       xorq    %rdx, %rdi
        movq    %rdi, %rax
-       sarq    %cl, %rax
+       subq    %rdx, %rax
        ret
        .p2align 4,,10
        .p2align 3

with -O2, tests attached.
Yes it's worse.  I thought it was better to avoid undefined behaviour at 
all prices to avoid "bad surprises".

Mikael




typedef signed long long HOST_WIDE_INT;

#define HOST_BITS_PER_WIDE_INT (8 * sizeof(HOST_WIDE_INT))

#define HOST_WIDE_INT_C(X) X ## LL
#define HOST_WIDE_INT_UC(X) HOST_WIDE_INT_C (X ## U)
#define HOST_WIDE_INT_1 HOST_WIDE_INT_C (1)
#define HOST_WIDE_INT_1U HOST_WIDE_INT_UC (1)
#define HOST_WIDE_INT_M1 HOST_WIDE_INT_C (-1)
#define HOST_WIDE_INT_M1U HOST_WIDE_INT_UC (-1)

#define gcc_checking_assert(test)

HOST_WIDE_INT
sext_hwi (HOST_WIDE_INT src, unsigned int prec)
{
  if (prec == HOST_BITS_PER_WIDE_INT)
    return src;
  else
    {
      gcc_checking_assert (prec < HOST_BITS_PER_WIDE_INT);
      int shift = HOST_BITS_PER_WIDE_INT - prec;
      return (src << shift) >> shift;
    }
}




typedef signed long long HOST_WIDE_INT;

#define HOST_BITS_PER_WIDE_INT (8 * sizeof(HOST_WIDE_INT))

#define HOST_WIDE_INT_C(X) X ## LL
#define HOST_WIDE_INT_UC(X) HOST_WIDE_INT_C (X ## U)
#define HOST_WIDE_INT_1 HOST_WIDE_INT_C (1)
#define HOST_WIDE_INT_1U HOST_WIDE_INT_UC (1)
#define HOST_WIDE_INT_M1 HOST_WIDE_INT_C (-1)
#define HOST_WIDE_INT_M1U HOST_WIDE_INT_UC (-1)

#define gcc_checking_assert(test)

HOST_WIDE_INT
sext_hwi (HOST_WIDE_INT src, unsigned int prec)
{
  if (prec == HOST_BITS_PER_WIDE_INT)
    return src;
  else
    {
      gcc_checking_assert (prec < HOST_BITS_PER_WIDE_INT);
      HOST_WIDE_INT sign_mask = HOST_WIDE_INT_1 << (prec - 1);
      HOST_WIDE_INT value_mask = (HOST_WIDE_INT_1U << prec) - HOST_WIDE_INT_1U;
      return (((src & value_mask) ^ sign_mask) - sign_mask);
    }
}