RFC: PR rtl-optimization/52876: Sign extend 32 to 64bit then clear upper 32bits fails O1 or higher

[H.J. Lu]

On Thu, Apr 5, 2012 at 11:48 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> CSE turns (reg:DI 64)
>
> (insn 6 3 7 2 (set (reg:DI 64)
>        (sign_extend:DI (subreg/u:SI (reg/v/f:DI 63 [ addr ]) 0))) x.i:6 122
> {*extendsidi2_rex64} (nil))
>
> into (reg/f:DI 64 [ addr ]).  But nonzero_bits1 in rtlanal.c has
>
> #if defined(POINTERS_EXTEND_UNSIGNED) && !defined(HAVE_ptr_extend)
>      /* If pointers extend unsigned and this is a pointer in Pmode, say that
>         all the bits above ptr_mode are known to be zero.  */
>      /* As we do not know which address space the pointer is refering to,
>         we can do this only if the target does not support different pointer
>         or address modes depending on the address space.  */
>      if (target_default_pointer_address_modes_p ()
>          && POINTERS_EXTEND_UNSIGNED && GET_MODE (x) == Pmode
>          && REG_POINTER (x))
>        nonzero &= GET_MODE_MASK (ptr_mode);
> #endif
>
> Setting REG_POINTER with incompatible pointer sign extension is wrong.
> This patch adds a bool parameter to set_reg_attrs_from_value to
> indicate if a register can be marked as a pointer.  Does it look OK?
>
> Thanks.
>
>
> H.J.
> ---
> gcc/
>
> 2012-04-05  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR rtl-optimization/52876
>        * emit-rtl.c (set_reg_attrs_from_value): Add a bool parameter
>        to indicate if a register can be a pointer.
>        (gen_reg_rtx_and_attrs): Pass true to set_reg_attrs_from_value.
>        (set_reg_attrs_for_parm): Likewise.
>
>        * emit-rtl.h (set_reg_attrs_from_value): Add a bool parameter.
>
>        * reginfo.c (reg_scan_mark_refs): Pass false to
>        set_reg_attrs_from_value with incompatible pointer sign
>        extension.
>
> gcc/testsuite
>
> 2012-04-05  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR rtl-optimization/52876
>        * gcc.target/i386/pr52876.c: New.
>
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index 8d7d441..791ff5c 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -967,7 +967,7 @@ adjust_reg_mode (rtx reg, enum machine_mode mode)
>    have different modes, REG is a (possibly paradoxical) lowpart of X.  */
>
>  void
> -set_reg_attrs_from_value (rtx reg, rtx x)
> +set_reg_attrs_from_value (rtx reg, rtx x, bool can_be_reg_pointer)
>  {
>   int offset;
>
> @@ -983,14 +983,14 @@ set_reg_attrs_from_value (rtx reg, rtx x)
>       if (MEM_OFFSET_KNOWN_P (x))
>        REG_ATTRS (reg) = get_reg_attrs (MEM_EXPR (x),
>                                         MEM_OFFSET (x) + offset);
> -      if (MEM_POINTER (x))
> +      if (can_be_reg_pointer && MEM_POINTER (x))
>        mark_reg_pointer (reg, 0);
>     }
>   else if (REG_P (x))
>     {
>       if (REG_ATTRS (x))
>        update_reg_offset (reg, x, offset);
> -      if (REG_POINTER (x))
> +      if (can_be_reg_pointer && REG_POINTER (x))
>        mark_reg_pointer (reg, REGNO_POINTER_ALIGN (REGNO (x)));
>     }
>  }
> @@ -1002,7 +1002,7 @@ rtx
>  gen_reg_rtx_and_attrs (rtx x)
>  {
>   rtx reg = gen_reg_rtx (GET_MODE (x));
> -  set_reg_attrs_from_value (reg, x);
> +  set_reg_attrs_from_value (reg, x, true);
>   return reg;
>  }
>
> @@ -1013,7 +1013,7 @@ void
>  set_reg_attrs_for_parm (rtx parm_rtx, rtx mem)
>  {
>   if (REG_P (parm_rtx))
> -    set_reg_attrs_from_value (parm_rtx, mem);
> +    set_reg_attrs_from_value (parm_rtx, mem, true);
>   else if (GET_CODE (parm_rtx) == PARALLEL)
>     {
>       /* Check for a NULL entry in the first slot, used to indicate that the
> diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
> index bc91193..2f259ef 100644
> --- a/gcc/emit-rtl.h
> +++ b/gcc/emit-rtl.h
> @@ -63,7 +63,7 @@ extern rtx copy_insn_1 (rtx);
>  extern rtx copy_insn (rtx);
>  extern rtx gen_int_mode (HOST_WIDE_INT, enum machine_mode);
>  extern rtx emit_copy_of_insn_after (rtx, rtx);
> -extern void set_reg_attrs_from_value (rtx, rtx);
> +extern void set_reg_attrs_from_value (rtx, rtx, bool);
>  extern void set_reg_attrs_for_parm (rtx, rtx);
>  extern void set_reg_attrs_for_decl_rtl (tree t, rtx x);
>  extern void adjust_reg_mode (rtx, enum machine_mode);
> diff --git a/gcc/reginfo.c b/gcc/reginfo.c
> index 6353126..585d7a8 100644
> --- a/gcc/reginfo.c
> +++ b/gcc/reginfo.c
> @@ -1224,14 +1224,24 @@ reg_scan_mark_refs (rtx x, rtx insn)
>       if (REG_P (dest) && !REG_ATTRS (dest))
>        {
>          rtx src = SET_SRC (x);
> +         bool can_be_reg_pointer = true;
>
>          while (GET_CODE (src) == SIGN_EXTEND
>                 || GET_CODE (src) == ZERO_EXTEND
>                 || GET_CODE (src) == TRUNCATE
>                 || (GET_CODE (src) == SUBREG && subreg_lowpart_p (src)))
> -           src = XEXP (src, 0);
> +           {
> +#if defined(POINTERS_EXTEND_UNSIGNED) && !defined(HAVE_ptr_extend)
> +             if ((GET_CODE (src) == SIGN_EXTEND
> +                  && POINTERS_EXTEND_UNSIGNED)
> +                 || (GET_CODE (src) != SIGN_EXTEND
> +                     && ! POINTERS_EXTEND_UNSIGNED))
> +               can_be_reg_pointer = false;
> +#endif
> +             src = XEXP (src, 0);
> +           }
>
> -         set_reg_attrs_from_value (dest, src);
> +         set_reg_attrs_from_value (dest, src, can_be_reg_pointer);
>        }
>
>       /* ... fall through ...  */
> diff --git a/gcc/testsuite/gcc.target/i386/pr52876.c b/gcc/testsuite/gcc.target/i386/pr52876.c
> new file mode 100644
> index 0000000..6d5e47a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr52876.c
> @@ -0,0 +1,25 @@
> +/* { dg-do run { target { x32 } } } */
> +/* { dg-options "-O2 -mx32 -maddress-mode=long" } */
> +
> +extern void abort (void);
> +
> +long long li;
> +
> +long long
> +__attribute__ ((noinline))
> +testfunc (void* addr)
> +{
> +  li = (long long)(int)addr;
> +  li &= 0xffffffff;
> +  return li;
> +}
> +
> +int main (void)
> +{
> +  volatile long long rv_test;
> +  rv_test = testfunc((void*)0x87651234);
> +  if (rv_test != 0x87651234ULL)
> +    abort ();
> +
> +  return 0;
> +}

PING.

-- 
H.J.