* [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865)
@ 2010-12-09 22:23 Jakub Jelinek
2010-12-10 9:57 ` Richard Guenther
2010-12-10 12:56 ` Chung-Lin Tang
0 siblings, 2 replies; 5+ messages in thread
From: Jakub Jelinek @ 2010-12-09 22:23 UTC (permalink / raw)
To: gcc-patches
Hi!
The last operand of ASM_OPERANDS and ASM_INPUT is a RTL locator,
for which we unfortunately don't guarantee uniqueness. One has
to call locator_eq to actually compare them.
On the testcases below, without -save-temps the locators are equal
while without them some other location_t got a locator in between.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
For 4.7 I guess it would be better to invent a new RTL format letter
and use it for the locators, but guess that would be too invasive now in
stage3.
2010-12-09 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/46865
* rtl.c (rtx_equal_p_cb, rtx_equal_p): For last operand of
ASM_OPERANDS and ASM_INPUT if integers are different,
call locator_eq.
* jump.c (rtx_renumbered_equal_p): Likewise.
* gcc.target/i386/pr46865-1.c: New test.
* gcc.target/i386/pr46865-2.c: New test.
--- gcc/rtl.c.jj 2010-11-01 09:07:23.000000000 +0100
+++ gcc/rtl.c 2010-12-09 19:35:34.000000000 +0100
@@ -431,7 +431,15 @@ rtx_equal_p_cb (const_rtx x, const_rtx y
case 'n':
case 'i':
if (XINT (x, i) != XINT (y, i))
- return 0;
+ {
+#ifndef GENERATOR_FILE
+ if (((code == ASM_OPERANDS && i == 6)
+ || (code == ASM_INPUT && i == 1))
+ && locator_eq (XINT (x, i), XINT (y, i)))
+ break;
+#endif
+ return 0;
+ }
break;
case 'V':
@@ -555,7 +563,15 @@ rtx_equal_p (const_rtx x, const_rtx y)
case 'n':
case 'i':
if (XINT (x, i) != XINT (y, i))
- return 0;
+ {
+#ifndef GENERATOR_FILE
+ if (((code == ASM_OPERANDS && i == 6)
+ || (code == ASM_INPUT && i == 1))
+ && locator_eq (XINT (x, i), XINT (y, i)))
+ break;
+#endif
+ return 0;
+ }
break;
case 'V':
--- gcc/jump.c.jj 2010-12-02 11:51:31.000000000 +0100
+++ gcc/jump.c 2010-12-09 19:30:09.000000000 +0100
@@ -1727,7 +1727,13 @@ rtx_renumbered_equal_p (const_rtx x, con
case 'i':
if (XINT (x, i) != XINT (y, i))
- return 0;
+ {
+ if (((code == ASM_OPERANDS && i == 6)
+ || (code == ASM_INPUT && i == 1))
+ && locator_eq (XINT (x, i), XINT (y, i)))
+ break;
+ return 0;
+ }
break;
case 't':
--- gcc/testsuite/gcc.target/i386/pr46865-1.c.jj 2010-12-09 19:58:10.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/pr46865-1.c 2010-12-09 19:58:06.000000000 +0100
@@ -0,0 +1,31 @@
+/* PR rtl-optimization/46865 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+extern unsigned long f;
+
+#define m1(f) \
+ if (f & 1) \
+ asm volatile ("nop /* asmnop */\n"); \
+ else \
+ asm volatile ("nop /* asmnop */\n");
+
+#define m2(f) \
+ if (f & 1) \
+ asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx"); \
+ else \
+ asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx");
+
+void
+foo (void)
+{
+ m1 (f);
+}
+
+void
+bar (void)
+{
+ m2 (f);
+}
+
+/* { dg-final { scan-assembler-times "asmnop" 2 } } */
--- gcc/testsuite/gcc.target/i386/pr46865-2.c.jj 2010-12-09 19:58:13.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/pr46865-2.c 2010-12-09 19:57:40.000000000 +0100
@@ -0,0 +1,32 @@
+/* PR rtl-optimization/46865 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -save-temps" } */
+
+extern unsigned long f;
+
+#define m1(f) \
+ if (f & 1) \
+ asm volatile ("nop /* asmnop */\n"); \
+ else \
+ asm volatile ("nop /* asmnop */\n");
+
+#define m2(f) \
+ if (f & 1) \
+ asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx"); \
+ else \
+ asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx");
+
+void
+foo (void)
+{
+ m1 (f);
+}
+
+void
+bar (void)
+{
+ m2 (f);
+}
+
+/* { dg-final { scan-assembler-times "asmnop" 2 } } */
+/* { dg-final { cleanup-saved-temps } } */
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865)
2010-12-09 22:23 [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865) Jakub Jelinek
@ 2010-12-10 9:57 ` Richard Guenther
2010-12-10 12:56 ` Chung-Lin Tang
1 sibling, 0 replies; 5+ messages in thread
From: Richard Guenther @ 2010-12-10 9:57 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Thu, Dec 9, 2010 at 11:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The last operand of ASM_OPERANDS and ASM_INPUT is a RTL locator,
> for which we unfortunately don't guarantee uniqueness. One has
> to call locator_eq to actually compare them.
> On the testcases below, without -save-temps the locators are equal
> while without them some other location_t got a locator in between.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> For 4.7 I guess it would be better to invent a new RTL format letter
> and use it for the locators, but guess that would be too invasive now in
> stage3.
I agree.
Ok.
Thanks,
Richard.
> 2010-12-09 Jakub Jelinek <jakub@redhat.com>
>
> PR rtl-optimization/46865
> * rtl.c (rtx_equal_p_cb, rtx_equal_p): For last operand of
> ASM_OPERANDS and ASM_INPUT if integers are different,
> call locator_eq.
> * jump.c (rtx_renumbered_equal_p): Likewise.
>
> * gcc.target/i386/pr46865-1.c: New test.
> * gcc.target/i386/pr46865-2.c: New test.
>
> --- gcc/rtl.c.jj 2010-11-01 09:07:23.000000000 +0100
> +++ gcc/rtl.c 2010-12-09 19:35:34.000000000 +0100
> @@ -431,7 +431,15 @@ rtx_equal_p_cb (const_rtx x, const_rtx y
> case 'n':
> case 'i':
> if (XINT (x, i) != XINT (y, i))
> - return 0;
> + {
> +#ifndef GENERATOR_FILE
> + if (((code == ASM_OPERANDS && i == 6)
> + || (code == ASM_INPUT && i == 1))
> + && locator_eq (XINT (x, i), XINT (y, i)))
> + break;
> +#endif
> + return 0;
> + }
> break;
>
> case 'V':
> @@ -555,7 +563,15 @@ rtx_equal_p (const_rtx x, const_rtx y)
> case 'n':
> case 'i':
> if (XINT (x, i) != XINT (y, i))
> - return 0;
> + {
> +#ifndef GENERATOR_FILE
> + if (((code == ASM_OPERANDS && i == 6)
> + || (code == ASM_INPUT && i == 1))
> + && locator_eq (XINT (x, i), XINT (y, i)))
> + break;
> +#endif
> + return 0;
> + }
> break;
>
> case 'V':
> --- gcc/jump.c.jj 2010-12-02 11:51:31.000000000 +0100
> +++ gcc/jump.c 2010-12-09 19:30:09.000000000 +0100
> @@ -1727,7 +1727,13 @@ rtx_renumbered_equal_p (const_rtx x, con
>
> case 'i':
> if (XINT (x, i) != XINT (y, i))
> - return 0;
> + {
> + if (((code == ASM_OPERANDS && i == 6)
> + || (code == ASM_INPUT && i == 1))
> + && locator_eq (XINT (x, i), XINT (y, i)))
> + break;
> + return 0;
> + }
> break;
>
> case 't':
> --- gcc/testsuite/gcc.target/i386/pr46865-1.c.jj 2010-12-09 19:58:10.000000000 +0100
> +++ gcc/testsuite/gcc.target/i386/pr46865-1.c 2010-12-09 19:58:06.000000000 +0100
> @@ -0,0 +1,31 @@
> +/* PR rtl-optimization/46865 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +extern unsigned long f;
> +
> +#define m1(f) \
> + if (f & 1) \
> + asm volatile ("nop /* asmnop */\n"); \
> + else \
> + asm volatile ("nop /* asmnop */\n");
> +
> +#define m2(f) \
> + if (f & 1) \
> + asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx"); \
> + else \
> + asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx");
> +
> +void
> +foo (void)
> +{
> + m1 (f);
> +}
> +
> +void
> +bar (void)
> +{
> + m2 (f);
> +}
> +
> +/* { dg-final { scan-assembler-times "asmnop" 2 } } */
> --- gcc/testsuite/gcc.target/i386/pr46865-2.c.jj 2010-12-09 19:58:13.000000000 +0100
> +++ gcc/testsuite/gcc.target/i386/pr46865-2.c 2010-12-09 19:57:40.000000000 +0100
> @@ -0,0 +1,32 @@
> +/* PR rtl-optimization/46865 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -save-temps" } */
> +
> +extern unsigned long f;
> +
> +#define m1(f) \
> + if (f & 1) \
> + asm volatile ("nop /* asmnop */\n"); \
> + else \
> + asm volatile ("nop /* asmnop */\n");
> +
> +#define m2(f) \
> + if (f & 1) \
> + asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx"); \
> + else \
> + asm volatile ("nop /* asmnop */\n" : : "i" (6) : "cx");
> +
> +void
> +foo (void)
> +{
> + m1 (f);
> +}
> +
> +void
> +bar (void)
> +{
> + m2 (f);
> +}
> +
> +/* { dg-final { scan-assembler-times "asmnop" 2 } } */
> +/* { dg-final { cleanup-saved-temps } } */
>
> Jakub
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865)
2010-12-09 22:23 [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865) Jakub Jelinek
2010-12-10 9:57 ` Richard Guenther
@ 2010-12-10 12:56 ` Chung-Lin Tang
2010-12-10 13:35 ` Jakub Jelinek
1 sibling, 1 reply; 5+ messages in thread
From: Chung-Lin Tang @ 2010-12-10 12:56 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On 2010/12/10 06:06, Jakub Jelinek wrote:
> The last operand of ASM_OPERANDS and ASM_INPUT is a RTL locator,
> for which we unfortunately don't guarantee uniqueness. One has
> to call locator_eq to actually compare them.
> On the testcases below, without -save-temps the locators are equal
> while without them some other location_t got a locator in between.
Actually, I think the issue is not just about comparing locators, but
rather: why do you need locator equality for ASM_INPUT/ASM_OPERANDS to
be equivalent? The checking of the other fields should be sufficient.
This way for the testcase, the non-macro version would then also be
combined into a single asm.
I was about to submit this patch, which simply skips the ending 'i'
field for ASM_INPUT/ASM_OPERANDS when checking equivalence. Bootstrapped
and tested on i686 and x86_64; tested on ARM-Linux with QEMU.
Thanks,
Chung-Lin
2010-12-10 Chung-Lin Tang <cltang@codesourcery.com>
PR rtl-optimization/46865
* rtl.c (rtx_equal_p_cb): Skip location operand when comparing
for equivalence.
(rtx_equal_p): Same.
* jump.c (rtx_renumbered_equal_p): Same.
[-- Attachment #2: asm.diff --]
[-- Type: text/plain, Size: 1663 bytes --]
Index: rtl.c
===================================================================
--- rtl.c (revision 167678)
+++ rtl.c (working copy)
@@ -360,6 +360,7 @@
int j;
enum rtx_code code;
const char *fmt;
+ int fmt_end;
rtx nx, ny;
if (x == y)
@@ -419,7 +420,10 @@
fail to match, return 0 for the whole thing. */
fmt = GET_RTX_FORMAT (code);
- for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
+ fmt_end = (GET_RTX_LENGTH (code)
+ - (code == ASM_INPUT || code == ASM_OPERANDS ? 2 : 1));
+
+ for (i = fmt_end; i >= 0; i--)
{
switch (fmt[i])
{
@@ -490,6 +494,7 @@
int j;
enum rtx_code code;
const char *fmt;
+ int fmt_end;
if (x == y)
return 1;
@@ -543,7 +548,10 @@
fail to match, return 0 for the whole thing. */
fmt = GET_RTX_FORMAT (code);
- for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
+ fmt_end = (GET_RTX_LENGTH (code)
+ - (code == ASM_INPUT || code == ASM_OPERANDS ? 2 : 1));
+
+ for (i = fmt_end; i >= 0; i--)
{
switch (fmt[i])
{
Index: jump.c
===================================================================
--- jump.c (revision 167678)
+++ jump.c (working copy)
@@ -1584,6 +1584,7 @@
int i;
const enum rtx_code code = GET_CODE (x);
const char *fmt;
+ int fmt_end;
if (x == y)
return 1;
@@ -1715,7 +1716,10 @@
fail to match, return 0 for the whole things. */
fmt = GET_RTX_FORMAT (code);
- for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
+ fmt_end = (GET_RTX_LENGTH (code)
+ - (code == ASM_INPUT || code == ASM_OPERANDS ? 2 : 1));
+
+ for (i = fmt_end; i >= 0; i--)
{
int j;
switch (fmt[i])
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865)
2010-12-10 12:56 ` Chung-Lin Tang
@ 2010-12-10 13:35 ` Jakub Jelinek
2010-12-10 14:30 ` Chung-Lin Tang
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2010-12-10 13:35 UTC (permalink / raw)
To: Chung-Lin Tang; +Cc: gcc-patches
On Fri, Dec 10, 2010 at 08:50:44PM +0800, Chung-Lin Tang wrote:
> On 2010/12/10 06:06, Jakub Jelinek wrote:
> >The last operand of ASM_OPERANDS and ASM_INPUT is a RTL locator,
> >for which we unfortunately don't guarantee uniqueness. One has
> >to call locator_eq to actually compare them.
> >On the testcases below, without -save-temps the locators are equal
> >while without them some other location_t got a locator in between.
>
> Actually, I think the issue is not just about comparing locators,
> but rather: why do you need locator equality for
> ASM_INPUT/ASM_OPERANDS to be equivalent? The checking of the other
Because then you regress in debug info quality, we emit the location
with .loc before the asm. Though, for -O0 we hopefully don't perform
crossjumping and similar optimizations, and for -O1+ the code quality is
more important than debug info quality.
If we'd want to ignore it, the right fix wouldn't be your patch,
but just making that operand use 0 format instead of i and adjust
print-rtl.c etc.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865)
2010-12-10 13:35 ` Jakub Jelinek
@ 2010-12-10 14:30 ` Chung-Lin Tang
0 siblings, 0 replies; 5+ messages in thread
From: Chung-Lin Tang @ 2010-12-10 14:30 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
Jakub Jelinek wrote:
>> Actually, I think the issue is not just about comparing locators,
>> > but rather: why do you need locator equality for
>> > ASM_INPUT/ASM_OPERANDS to be equivalent? The checking of the other
> Because then you regress in debug info quality, we emit the location
> with .loc before the asm. Though, for -O0 we hopefully don't perform
> crossjumping and similar optimizations, and for -O1+ the code quality is
> more important than debug info quality.
>
> If we'd want to ignore it, the right fix wouldn't be your patch,
> but just making that operand use 0 format instead of i and adjust
> print-rtl.c etc.
I see your point.
However, to adjust the equivalence criteria depending on optimization
level (as you suggested), changing to '0' format does not make it easier
:) I would still suggest using code to achieve that.
CL
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-12-10 13:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-09 22:23 [PATCH] Fix rtx_equal_p and similar predicates on ASM_OPERANDS/ASM_INPUT (PR rtl-optimization/46865) Jakub Jelinek
2010-12-10 9:57 ` Richard Guenther
2010-12-10 12:56 ` Chung-Lin Tang
2010-12-10 13:35 ` Jakub Jelinek
2010-12-10 14:30 ` Chung-Lin Tang
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).