From: "H.J. Lu" <hjl.tools@gmail.com>
To: "H.J. Lu" <hjl.tools@gmail.com>,
gcc-patches@gcc.gnu.org, Uros Bizjak <ubizjak@gmail.com>,
rdsandiford@googlemail.com
Subject: Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures
Date: Wed, 08 Aug 2012 13:40:00 -0000 [thread overview]
Message-ID: <CAMe9rOrtf40ViaoK=W+XVZ543dp8M-w7VM142GYU9C_+DbKATQ@mail.gmail.com> (raw)
In-Reply-To: <g4fw7xiz5l.fsf@richards-thinkpad.stglab.manchester.uk.ibm.com>
On Wed, Aug 8, 2012 at 1:08 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> diff --git a/gcc/combine.c b/gcc/combine.c
>> index a07c046..b9a3589 100644
>> --- a/gcc/combine.c
>> +++ b/gcc/combine.c
>> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx
>> x)
>> if (omode == imode)
>> return x;
>>
>> - /* Return identity if this is a CONST or symbolic reference. */
>> - if (omode == Pmode
>> - && (GET_CODE (x) == CONST
>> - || GET_CODE (x) == SYMBOL_REF
>> - || GET_CODE (x) == LABEL_REF))
>> - return x;
>> + if (omode == Pmode)
>> + {
>> + /* Return identity if this is a symbolic reference. */
>> + if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
>> + return x;
>> +
>> + /* Return identity for CONST unless this is a PLUS of 2 constant
>> + operands. */
>> + if (GET_CODE (x) == CONST)
>> + {
>> + rtx y = XEXP (x, 0);
>> + if (GET_CODE (y) == PLUS
>> + && ((CONST_INT_P (XEXP (y, 1))
>> + && (GET_CODE (XEXP (y, 0)) == CONST
>> + || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>> + || GET_CODE (XEXP (y, 0)) == LABEL_REF))
>> + || (CONST_INT_P (XEXP (y, 1))
>> + && (GET_CODE (XEXP (y, 0)) == CONST
>> + || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
>> + || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
>> + goto fail;
>> + return x;
>> + }
>> + }
>>
>> /* We can only support MODE being wider than a word if X is a
>> constant integer or has a mode the same size. */
>>
>> works for the testcase.
>
> My point was that the "return x" is always wrong. Whenever we return x
> here, we know we're returning something in a different mode from the one
> that the caller wanted. Returning a Pmode LABEL_REF might not trigger
> that plus_constant assert, but it's still wrong.
>
> It looks like this came from the mips-rewrite branch:
>
> 2003-03-13 Richard Sandiford <rsandifo@redhat.com>
>
> * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
> a CONST as identity. Check the return value of gen_lowpart_common.
>
> so I can categorically confirm that the person who wrote it didn't
> know what they were doing. It also means that this case was motivated
> by an experiment to make Pmode == DImode for n32, which we ended up
> discarding because it produced worse code.
>
> If this case really is important, we might consider using
> convert_memory_address (Pmode, x) instead. I'm not sure whether
> that would be right for targets with address spaces though, because
> we don't know which address space is associated with the address.
> Hopefully someone who knows address spaces can comment.
>
> If it is correct, then it should probably go in gen_lowpart_common
> rather than gen_lowpart_for_combine.
>
> But given how few people have hit this, it doesn't look like a
> particularly important attempted optimisation. I'll pre-approve
> a patch that undoes my mistake and simply removes:
>
> /* Return identity if this is a CONST or symbolic reference. */
> if (omode == Pmode
> && (GET_CODE (x) == CONST
> || GET_CODE (x) == SYMBOL_REF
> || GET_CODE (x) == LABEL_REF))
> return x;
>
> Richard
This is the patch I checked in.
Thanks.
--
H.J.
---
gcc/
2012-08-08 Richard Sandiford <rdsandiford@googlemail.com>
H.J. Lu <hongjiu.lu@intel.com>
PR rtl-optimization/54157
* combine.c (gen_lowpart_for_combine): Don't return identity
for CONST or symbolic reference.
gcc/testsuite/
2012-08-08 H.J. Lu <hongjiu.lu@intel.com>
PR rtl-optimization/54157
* gcc.target/i386/pr54157.c: New file.
diff --git a/gcc/combine.c b/gcc/combine.c
index 495e129..2b91eb9 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10634,13 +10634,6 @@ gen_lowpart_for_combine (enum machine_mode
omode, rtx x)
if (omode == imode)
return x;
- /* Return identity if this is a CONST or symbolic reference. */
- if (omode == Pmode
- && (GET_CODE (x) == CONST
- || GET_CODE (x) == SYMBOL_REF
- || GET_CODE (x) == LABEL_REF))
- return x;
-
/* We can only support MODE being wider than a word if X is a
constant integer or has a mode the same size. */
if (GET_MODE_SIZE (omode) > UNITS_PER_WORD
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c
b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 0000000..b5c4528
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options "-O2 -mx32 -maddress-mode=long -ftree-vectorize" } */
+
+struct s2{
+ int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+ struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+ int i,j;
+
+ for (i = 0; i < 24 -4; i++)
+ for (j = 0; j < 24 -4; j++)
+ tmp2[2].e.n[1][i][j] = 8;
+}
next prev parent reply other threads:[~2012-08-08 13:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-01 18:41 H.J. Lu
2012-08-01 18:59 ` Richard Sandiford
2012-08-01 19:14 ` H.J. Lu
2012-08-02 18:21 ` H.J. Lu
2012-08-05 7:47 ` Richard Sandiford
2012-08-07 19:28 ` H.J. Lu
2012-08-08 8:09 ` Richard Sandiford
2012-08-08 13:40 ` H.J. Lu [this message]
2012-08-08 13:43 ` Uros Bizjak
2012-08-08 13:50 ` H.J. Lu
2012-08-08 15:11 ` Richard Sandiford
2012-08-09 14:51 ` H.J. Lu
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='CAMe9rOrtf40ViaoK=W+XVZ543dp8M-w7VM142GYU9C_+DbKATQ@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rdsandiford@googlemail.com \
--cc=ubizjak@gmail.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).