From: "H.J. Lu" <hjl.tools@gmail.com>
To: Eric Botcazou <ebotcazou@adacore.com>,
Richard Sandiford <rdsandiford@googlemail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: RFC: PR rtl-optimization/52876: Sign extend 32 to 64bit then clear upper 32bits fails O1 or higher
Date: Tue, 10 Apr 2012 14:49:00 -0000 [thread overview]
Message-ID: <CAMe9rOppadJuPnntovt1aM5HSuXsPzAGUTWRr0H1RTmEzFhmtw@mail.gmail.com> (raw)
In-Reply-To: <20120405184839.GA4858@intel.com>
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.
next prev parent reply other threads:[~2012-04-10 14:49 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-05 18:48 H.J. Lu
2012-04-10 14:49 ` H.J. Lu [this message]
2012-04-11 14:21 ` Richard Sandiford
2012-04-11 17:21 ` H.J. Lu
2012-04-11 18:53 ` Richard Sandiford
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMe9rOppadJuPnntovt1aM5HSuXsPzAGUTWRr0H1RTmEzFhmtw@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=ebotcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rdsandiford@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).