From: "H.J. Lu" <hjl.tools@gmail.com>
To: Eric Botcazou <ebotcazou@adacore.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: RFC: Add TARGET_EXPAND_COMPOUND_OPERATION
Date: Mon, 28 Jun 2010 18:57:00 -0000 [thread overview]
Message-ID: <AANLkTikT54zVLWftyRm1nwPfpjp1DF_EsRASskBqbidk@mail.gmail.com> (raw)
In-Reply-To: <AANLkTim5Y5uVq9ylpE58TxaokAFi508kC-N_4bsRn1Oj@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]
On Mon, Jun 28, 2010 at 10:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jun 28, 2010 at 7:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Jun 28, 2010 at 3:05 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>> However, combiner never exposes this to x86 backend. I added
>>>> a TARGET_EXPAND_COMPOUND_OPERATION hook to allow x86 backend to
>>>> optimize it. For
>>>>
>>>> ---
>>>> typedef struct
>>>> {
>>>> unsigned char c1;
>>>> unsigned char c2;
>>>> unsigned char c3;
>>>> unsigned char c4;
>>>> } foo_t;
>>>>
>>>> int
>>>> foo (foo_t x)
>>>> {
>>>> return x.c2 > 4;
>>>> }
>>>
>>> I think that disabling the canonicalization done by expand_compound_operation
>>> can disable certain forms of combining that are beneficial to x86 even for
>>> this kind of patterns.
>>>
>>> The combiner already knows that it needs to re-create the compound forms when
>>> it is trying to simplify a comparison, see simplify_comparison. The problem
>>> with your testcase is that make_compound_operation fails to do so.
>>>
>>> Lightly tested patch attached.
>>>
>>>
>>> * combine.c (make_compound_operation) <SUBREG>: Do not return the
>>> result of force_to_mode if it partially re-expanded the compound.
>>>
>>
>> It works on my testcases. I will do a full bootstrap and test on
>> Linux/x86-64.
>>
>
> There are no regressions. Can you install it?
>
Here is the complete patch with testcases.
Thanks.
--
H.J.
---
gcc/
2010-06-28 Eric Botcazou <ebotcazou@adacore.com>
PR rtl-optimization/44659
* combine.c (make_compound_operation) <SUBREG>: Do not return the
result of force_to_mode if it partially re-expanded the compound.
gcc/testsuite/
2010-06-28 H.J. Lu <hongjiu.lu@intel.com>
PR rtl-optimization/44659
* gcc.target/i386/extract-1.c: New.
* gcc.target/i386/extract-2.c: Likewise.
* gcc.target/i386/extract-3.c: Likewise.
* gcc.target/i386/extract-4.c: Likewise.
* gcc.target/i386/extract-5.c: Likewise.
* gcc.target/i386/extract-6.c: Likewise.
[-- Attachment #2: gcc-pr44659-2.patch --]
[-- Type: text/plain, Size: 6278 bytes --]
gcc/
2010-06-28 Eric Botcazou <ebotcazou@adacore.com>
PR rtl-optimization/44659
* combine.c (make_compound_operation) <SUBREG>: Do not return the
result of force_to_mode if it partially re-expanded the compound.
gcc/testsuite/
2010-06-28 H.J. Lu <hongjiu.lu@intel.com>
PR rtl-optimization/44659
* gcc.target/i386/extract-1.c: New.
* gcc.target/i386/extract-2.c: Likewise.
* gcc.target/i386/extract-3.c: Likewise.
* gcc.target/i386/extract-4.c: Likewise.
* gcc.target/i386/extract-5.c: Likewise.
* gcc.target/i386/extract-6.c: Likewise.
diff --git a/gcc/combine.c b/gcc/combine.c
index 1bee2c7..d3305cb 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -7277,22 +7277,21 @@ make_compound_operation (rtx x, enum rtx_code in_code)
/* Call ourselves recursively on the inner expression. If we are
narrowing the object and it has a different RTL code from
what it originally did, do this SUBREG as a force_to_mode. */
-
- tem = make_compound_operation (SUBREG_REG (x), in_code);
-
{
- rtx simplified = simplify_subreg (mode, tem, GET_MODE (SUBREG_REG (x)),
- SUBREG_BYTE (x));
+ rtx inner = SUBREG_REG (x), simplified;
+
+ tem = make_compound_operation (inner, in_code);
+ simplified
+ = simplify_subreg (mode, tem, GET_MODE (inner), SUBREG_BYTE (x));
if (simplified)
tem = simplified;
- if (GET_CODE (tem) != GET_CODE (SUBREG_REG (x))
- && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))
+ if (GET_CODE (tem) != GET_CODE (inner)
+ && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (inner))
&& subreg_lowpart_p (x))
{
- rtx newer = force_to_mode (tem, mode, ~(HOST_WIDE_INT) 0,
- 0);
+ rtx newer = force_to_mode (tem, mode, ~(HOST_WIDE_INT) 0, 0);
/* If we have something other than a SUBREG, we might have
done an expansion, so rerun ourselves. */
@@ -7300,9 +7299,16 @@ make_compound_operation (rtx x, enum rtx_code in_code)
newer = make_compound_operation (newer, in_code);
/* force_to_mode can expand compounds. If it just re-expanded the
- compound use gen_lowpart instead to convert to the desired
- mode. */
- if (rtx_equal_p (newer, x))
+ compound, use gen_lowpart to convert to the desired mode. */
+ if (rtx_equal_p (newer, x)
+ /* Likewise if it re-expanded the compound only partially.
+ This happens for SUBREG of ZERO_EXTRACT if they extract
+ the same number of bits. */
+ || (GET_CODE (newer) == SUBREG
+ && (GET_CODE (SUBREG_REG (newer)) == LSHIFTRT
+ || GET_CODE (SUBREG_REG (newer)) == ASHIFTRT)
+ && GET_CODE (inner) == AND
+ && rtx_equal_p (SUBREG_REG (newer), XEXP (inner, 0))))
return gen_lowpart (GET_MODE (x), tem);
return newer;
diff --git a/gcc/testsuite/gcc.target/i386/extract-1.c b/gcc/testsuite/gcc.target/i386/extract-1.c
new file mode 100644
index 0000000..102beb2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/extract-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic" } */
+
+int
+foo (unsigned char x, unsigned char y)
+{
+ return (x % y) != 0;
+}
+
+/* { dg-final { scan-assembler-not "test\[b\]?\[^\\n\]*%\[a-d\]l" } } */
diff --git a/gcc/testsuite/gcc.target/i386/extract-2.c b/gcc/testsuite/gcc.target/i386/extract-2.c
new file mode 100644
index 0000000..3bb5f15
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/extract-2.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic" } */
+
+int
+foo (unsigned char x, unsigned char y)
+{
+ return (x % y) > 4;
+}
+
+/* { dg-final { scan-assembler-times "cmp\[b\]?\[^\\n\]*%\[a-d\]h" 1 } } */
+/* { dg-final { scan-assembler-not "cmp\[b\]?\[^\\n\]*%\[a-d\]l" } } */
diff --git a/gcc/testsuite/gcc.target/i386/extract-3.c b/gcc/testsuite/gcc.target/i386/extract-3.c
new file mode 100644
index 0000000..520bf3b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/extract-3.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic" } */
+
+typedef struct
+{
+ unsigned char c1;
+ unsigned char c2;
+ unsigned char c3;
+ unsigned char c4;
+} foo_t;
+
+int
+#ifndef __x86_64__
+__attribute__((regparm(3)))
+#endif
+foo (foo_t x)
+{
+ return x.c2 != 0;
+}
+
+/* { dg-final { scan-assembler-not "test\[b\]?\[^\\n\]*%\[a-z0-9\]+l" } } */
diff --git a/gcc/testsuite/gcc.target/i386/extract-4.c b/gcc/testsuite/gcc.target/i386/extract-4.c
new file mode 100644
index 0000000..716ae22
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/extract-4.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic" } */
+
+typedef struct
+{
+ unsigned char c1;
+ unsigned char c2;
+ unsigned char c3;
+ unsigned char c4;
+} foo_t;
+
+int
+#ifndef __x86_64__
+__attribute__((regparm(3)))
+#endif
+foo (foo_t x)
+{
+ return x.c2 > 4;
+}
+
+/* { dg-final { scan-assembler-times "cmp\[b\]?\[^\\n\]*%\[a-z0-9\]+h" 1 } } */
+/* { dg-final { scan-assembler-not "cmp\[b\]?\[^\\n\]*%\[a-z0-9\]+l" } } */
diff --git a/gcc/testsuite/gcc.target/i386/extract-5.c b/gcc/testsuite/gcc.target/i386/extract-5.c
new file mode 100644
index 0000000..a488daf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/extract-5.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic" } */
+
+typedef struct
+{
+ unsigned int c1:8;
+ unsigned int c2:8;
+ unsigned int c3:8;
+ unsigned int c4:8;
+} foo_t;
+
+int
+#ifndef __x86_64__
+__attribute__((regparm(3)))
+#endif
+foo (foo_t x)
+{
+ return x.c2 != 0;
+}
+
+/* { dg-final { scan-assembler-not "test\[b\]?\[^\\n\]*%\[a-z0-9\]+l" } } */
diff --git a/gcc/testsuite/gcc.target/i386/extract-6.c b/gcc/testsuite/gcc.target/i386/extract-6.c
new file mode 100644
index 0000000..1440ec3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/extract-6.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=generic" } */
+
+typedef struct
+{
+ unsigned int c1:8;
+ unsigned int c2:8;
+ unsigned int c3:8;
+ unsigned int c4:8;
+
+} foo_t;
+
+int
+#ifndef __x86_64__
+__attribute__((regparm(3)))
+#endif
+foo (foo_t x)
+{
+ return x.c2 > 4;
+}
+
+/* { dg-final { scan-assembler-times "cmp\[b\]?\[^\\n\]*%\[a-z0-9\]+h" 1 } } */
+/* { dg-final { scan-assembler-not "cmp\[b\]?\[^\\n\]*%\[a-z0-9\]+l" } } */
next prev parent reply other threads:[~2010-06-28 17:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-25 23:29 H.J. Lu
2010-06-28 10:44 ` Eric Botcazou
2010-06-28 15:59 ` H.J. Lu
2010-06-28 18:39 ` H.J. Lu
2010-06-28 18:57 ` H.J. Lu [this message]
2010-06-29 12:05 ` Eric Botcazou
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=AANLkTikT54zVLWftyRm1nwPfpjp1DF_EsRASskBqbidk@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=ebotcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
/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).