PATCH: PR rtl-optimization/47502: Never combine asm statement

PATCH: PR rtl-optimization/47502: Never combine asm statement

[H.J. Lu]

It is a bad idea to combine asm statement.  This patch disallows it.
Any comments?

Thanks.


H.J.
---
commit a11b95180e53d3a0bc3eabad5594859cea7c3334
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Jan 27 18:12:27 2011 -0800

    Never combine asm statement.

diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32
index 0ae4761..ef65e7b 100644
--- a/gcc/ChangeLog.x32
+++ b/gcc/ChangeLog.x32
@@ -1,3 +1,8 @@
+2011-01-27  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl-optimization/47502
+	* combine.c (cant_combine_insn_p): Never combine asm statement.
+
 2011-01-25  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR target/47446
diff --git a/gcc/combine.c b/gcc/combine.c
index 3ee53e6..d4a8079 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2122,13 +2122,14 @@ cant_combine_insn_p (rtx insn)
   /* Never combine loads and stores involving hard regs that are likely
      to be spilled.  The register allocator can usually handle such
      reg-reg moves by tying.  If we allow the combiner to make
-     substitutions of likely-spilled regs, reload might die.
+     substitutions of likely-spilled regs, reload might die.  Never
+     combine asm statement.
      As an exception, we allow combinations involving fixed regs; these are
      not available to the register allocator so there's no risk involved.  */
 
   set = single_set (insn);
   if (! set)
-    return 0;
+    return asm_noperands (PATTERN (insn)) > 0;
   src = SET_SRC (set);
   dest = SET_DEST (set);
   if (GET_CODE (src) == SUBREG)
@@ -2144,7 +2145,7 @@ cant_combine_insn_p (rtx insn)
 	      && targetm.class_likely_spilled_p (REGNO_REG_CLASS (REGNO (dest))))))
     return 1;
 
-  return 0;
+  return asm_noperands (src) > 0;
 }
 
 struct likely_spilled_retval_info
diff --git a/gcc/testsuite/ChangeLog.x32 b/gcc/testsuite/ChangeLog.x32
index 597294e..8759881 100644
--- a/gcc/testsuite/ChangeLog.x32
+++ b/gcc/testsuite/ChangeLog.x32
@@ -1,3 +1,9 @@
+2011-01-27  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl-optimization/47502
+	* gcc.target/i386/pr47502-1.c: New.
+	* gcc.target/i386/pr47502-2.c: Likewise.
+
 2011-01-25  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR target/47446
diff --git a/gcc/testsuite/gcc.target/i386/pr47502-1.c b/gcc/testsuite/gcc.target/i386/pr47502-1.c
new file mode 100644
index 0000000..727afe9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47502-1.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+void
+foo (const void *xxxxx, void *yyyyy, long y)
+{
+  asm volatile ("" :: "c" ((xxxxx)), "d" ((yyyyy)), "S" (y));
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr47502-2.c b/gcc/testsuite/gcc.target/i386/pr47502-2.c
new file mode 100644
index 0000000..1f57ea0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47502-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int
+foo (int how, const void *set, void *oset)
+{
+  int resultvar;
+  asm volatile (""
+                : "=a" (resultvar)
+                : "0" (14) , "b" (how), "c" ((set)), "d" ((oset)), "S" (65 / 8) : "memory", "cc");
+  return resultvar;
+}

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[H.J. Lu]

Hi Eric,

Is this patch OK for trunk?

Thanks.


H.J.
---
On Thu, Jan 27, 2011 at 6:14 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> It is a bad idea to combine asm statement.  This patch disallows it.
> Any comments?
>
> Thanks.
>
>
> H.J.
> ---
> commit a11b95180e53d3a0bc3eabad5594859cea7c3334
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Thu Jan 27 18:12:27 2011 -0800
>
>    Never combine asm statement.
>
> diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32
> index 0ae4761..ef65e7b 100644
> --- a/gcc/ChangeLog.x32
> +++ b/gcc/ChangeLog.x32
> @@ -1,3 +1,8 @@
> +2011-01-27  H.J. Lu  <hongjiu.lu@intel.com>
> +
> +       PR rtl-optimization/47502
> +       * combine.c (cant_combine_insn_p): Never combine asm statement.
> +
>  2011-01-25  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR target/47446
> diff --git a/gcc/combine.c b/gcc/combine.c
> index 3ee53e6..d4a8079 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -2122,13 +2122,14 @@ cant_combine_insn_p (rtx insn)
>   /* Never combine loads and stores involving hard regs that are likely
>      to be spilled.  The register allocator can usually handle such
>      reg-reg moves by tying.  If we allow the combiner to make
> -     substitutions of likely-spilled regs, reload might die.
> +     substitutions of likely-spilled regs, reload might die.  Never
> +     combine asm statement.
>      As an exception, we allow combinations involving fixed regs; these are
>      not available to the register allocator so there's no risk involved.  */
>
>   set = single_set (insn);
>   if (! set)
> -    return 0;
> +    return asm_noperands (PATTERN (insn)) > 0;
>   src = SET_SRC (set);
>   dest = SET_DEST (set);
>   if (GET_CODE (src) == SUBREG)
> @@ -2144,7 +2145,7 @@ cant_combine_insn_p (rtx insn)
>              && targetm.class_likely_spilled_p (REGNO_REG_CLASS (REGNO (dest))))))
>     return 1;
>
> -  return 0;
> +  return asm_noperands (src) > 0;
>  }
>
>  struct likely_spilled_retval_info
> diff --git a/gcc/testsuite/ChangeLog.x32 b/gcc/testsuite/ChangeLog.x32
> index 597294e..8759881 100644
> --- a/gcc/testsuite/ChangeLog.x32
> +++ b/gcc/testsuite/ChangeLog.x32
> @@ -1,3 +1,9 @@
> +2011-01-27  H.J. Lu  <hongjiu.lu@intel.com>
> +
> +       PR rtl-optimization/47502
> +       * gcc.target/i386/pr47502-1.c: New.
> +       * gcc.target/i386/pr47502-2.c: Likewise.
> +
>  2011-01-25  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR target/47446
> diff --git a/gcc/testsuite/gcc.target/i386/pr47502-1.c b/gcc/testsuite/gcc.target/i386/pr47502-1.c
> new file mode 100644
> index 0000000..727afe9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr47502-1.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O" } */
> +
> +void
> +foo (const void *xxxxx, void *yyyyy, long y)
> +{
> +  asm volatile ("" :: "c" ((xxxxx)), "d" ((yyyyy)), "S" (y));
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr47502-2.c b/gcc/testsuite/gcc.target/i386/pr47502-2.c
> new file mode 100644
> index 0000000..1f57ea0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr47502-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +int
> +foo (int how, const void *set, void *oset)
> +{
> +  int resultvar;
> +  asm volatile (""
> +                : "=a" (resultvar)
> +                : "0" (14) , "b" (how), "c" ((set)), "d" ((oset)), "S" (65 / 8) : "memory", "cc");
> +  return resultvar;
> +}
>



-- 
H.J.

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[Geert Bosch]

On Mar 17, 2011, at 20:35, H.J. Lu wrote:

>> -     substitutions of likely-spilled regs, reload might die.
>> +     substitutions of likely-spilled regs, reload might die.  Never
>> +     combine asm statement.
> 
This has to be "statements", a plural.

  -Geert

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[H.J. Lu]

On Thu, Mar 17, 2011 at 8:28 PM, Geert Bosch <bosch@adacore.com> wrote:
>
> On Mar 17, 2011, at 20:35, H.J. Lu wrote:
>
>>> -     substitutions of likely-spilled regs, reload might die.
>>> +     substitutions of likely-spilled regs, reload might die.  Never
>>> +     combine asm statement.
>>
> This has to be "statements", a plural.
>

I will make the change.

Thanks.

-- 
H.J.

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[Eric Botcazou]

> Is this patch OK for trunk?

This has always worked for the other ports AFAIK so I don't think we should 
disable it without evaluating the impact on them.  If reload has so many 
problems with x32, maybe more fundamental changes should be made to the x86 
back-end to support it.

-- 
Eric Botcazou

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[H.J. Lu]

On Fri, Mar 18, 2011 at 12:51 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Is this patch OK for trunk?
>
> This has always worked for the other ports AFAIK so I don't think we should
> disable it without evaluating the impact on them.  If reload has so many
> problems with x32, maybe more fundamental changes should be made to the x86
> back-end to support it.
>

X32 port exposed many issues in GCC middle-end and RTL optimizations.
The other ports never generate such asm statements combine has to deal with.
Do you have any suggestions how to fix it in backend?

Thanks.


-- 
H.J.

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[Richard Henderson]

On 03/18/2011 01:56 PM, H.J. Lu wrote:
> On Fri, Mar 18, 2011 at 12:51 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> Is this patch OK for trunk?
>>
>> This has always worked for the other ports AFAIK so I don't think we should
>> disable it without evaluating the impact on them.  If reload has so many
>> problems with x32, maybe more fundamental changes should be made to the x86
>> back-end to support it.
>>
> 
> X32 port exposed many issues in GCC middle-end and RTL optimizations.
> The other ports never generate such asm statements combine has to deal with.
> Do you have any suggestions how to fix it in backend?

How about analyzing the problem properly?

All you posted was a patch, with no explanation of how the problem occurred.
Without that, no one can say whether this is the x32 port really tickling a
latent bug in the compiler, or whether it's a bug in your port.


r~

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[H.J. Lu]

On Fri, Mar 18, 2011 at 2:05 PM, Richard Henderson <rth@redhat.com> wrote:
> On 03/18/2011 01:56 PM, H.J. Lu wrote:
>> On Fri, Mar 18, 2011 at 12:51 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>> Is this patch OK for trunk?
>>>
>>> This has always worked for the other ports AFAIK so I don't think we should
>>> disable it without evaluating the impact on them.  If reload has so many
>>> problems with x32, maybe more fundamental changes should be made to the x86
>>> back-end to support it.
>>>
>>
>> X32 port exposed many issues in GCC middle-end and RTL optimizations.
>> The other ports never generate such asm statements combine has to deal with.
>> Do you have any suggestions how to fix it in backend?
>
> How about analyzing the problem properly?
>
> All you posted was a patch, with no explanation of how the problem occurred.
> Without that, no one can say whether this is the x32 port really tickling a
> latent bug in the compiler, or whether it's a bug in your port.
>

See analysis in:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47502


-- 
H.J.

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[Richard Henderson]

On 03/18/2011 02:51 PM, H.J. Lu wrote:
> See analysis in:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47502

You're patching the wrong place.  See can_combine_p,
which can test for specific sources, rather than 
cant_combine_insn_p which has no access to sources.


r~

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[H.J. Lu]

On Fri, Mar 18, 2011 at 3:39 PM, Richard Henderson <rth@redhat.com> wrote:
> On 03/18/2011 02:51 PM, H.J. Lu wrote:
>> See analysis in:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47502
>
> You're patching the wrong place.  See can_combine_p,
> which can test for specific sources, rather than
> cant_combine_insn_p which has no access to sources.
>

I can no longer trigger the failure. But I'd like to add those
2 testcases.  OK for trunk?

Thanks.


-- 
H.J.
---
2011-03-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/47502
	* gcc.target/i386/pr47502-1.c: New.
	* gcc.target/i386/pr47502-2.c: Likewise.

diff --git a/gcc/testsuite/gcc.target/i386/pr47502-1.c
b/gcc/testsuite/gcc.target/i386/pr47502-1.c
new file mode 100644
index 0000000..727afe9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47502-1.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+void
+foo (const void *xxxxx, void *yyyyy, long y)
+{
+  asm volatile ("" :: "c" ((xxxxx)), "d" ((yyyyy)), "S" (y));
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr47502-2.c
b/gcc/testsuite/gcc.target/i386/pr47502-2.c
new file mode 100644
index 0000000..1f57ea0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47502-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int
+foo (int how, const void *set, void *oset)
+{
+  int resultvar;
+  asm volatile (""
+                : "=a" (resultvar)
+                : "0" (14) , "b" (how), "c" ((set)), "d" ((oset)),
"S" (65 / 8) : "memory", "cc");
+  return resultvar;
+}

Re: PATCH: PR rtl-optimization/47502: Never combine asm statement

[Richard Henderson]

On 03/19/2011 08:29 AM, H.J. Lu wrote:
> 	PR rtl-optimization/47502
> 	* gcc.target/i386/pr47502-1.c: New.
> 	* gcc.target/i386/pr47502-2.c: Likewise.

Ok.


r~