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

[Richard Sandiford <rdsandiford at googlemail dot com>]

"H.J. Lu" <hongjiu.lu@intel.com> writes:
> 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.

Regardless of how it is currently used, I think gen_reg_rtx_and_attrs
(another caller to set_reg_attrs_from_value) is supposed to be a fairly
general function.  So rather than apply a fix specific to regscan,
I think it would be cleaner to redefine set_reg_attrs_from_value as
taking an arbitrary value.  The whole while loop:

	  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);

would then move to set_reg_attrs_from_value, and the "can be pointer"
thing hidden there.  The regscan code would just be:

      if (REG_P (dest) && !REG_ATTRS (dest))
	set_reg_attrs_from_value (dest, src);

Richard