This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Wed, Apr 11, 2012 at 7:21 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "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

Here is the updated patch to change set_reg_attrs_from_value to
take arbitrary value and check incompatible pointer sign
extension.  OK for trunk if there are no regressions on Linux/x86-64?

Thanks.


-- 
H.J.
---
gcc/

2012-04-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/52876
	* emit-rtl.c (set_reg_attrs_from_value): Handle arbitrary value.
	Don't call mark_reg_pointer for incompatible pointer sign
	extension.

	* reginfo.c (reg_scan_mark_refs): Call set_reg_attrs_from_value
	directly.

gcc/testsuite

2012-04-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/52876
	* gcc.target/i386/pr52876.c: New.

Attachment: gcc-pr52876-2.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]