Re: RFC: PR rtl-optimization/52876: Sign extend 32 to 64bit then clear upper 32bits fails O1 or higher
H.J. Lu hongjiu...@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...@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
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
rdsandif...@googlemail.com wrote:
H.J. Lu hongjiu...@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...@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...@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...@intel.com
PR rtl-optimization/52876
* gcc.target/i386/pr52876.c: New.
gcc-pr52876-2.patch
Description: Binary data
Re: RFC: PR rtl-optimization/52876: Sign extend 32 to 64bit then clear upper 32bits fails O1 or higher
H.J. Lu hjl.to...@gmail.com writes: 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? The new code ought to go before the HARD_REGISTER_P check, not after it. OK with that change, thanks. Richard
Re: RFC: PR rtl-optimization/52876: Sign extend 32 to 64bit then clear upper 32bits fails O1 or higher
On Thu, Apr 5, 2012 at 11:48 AM, H.J. Lu hongjiu...@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...@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...@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,
RFC: PR rtl-optimization/52876: Sign extend 32 to 64bit then clear upper 32bits fails O1 or higher
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...@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...@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
