[PATCH 4.8] PR66306 Fix reload problem with commutative operands

[PATCH 4.8] PR66306 Fix reload problem with commutative operands

[Andreas Krebbel]

Hi,

this fixes a reload problem with match_dup's on commutative operands.

Bootstrapped and regtested on x86-64, ppc64, and s390x.

Ok?

Bye,

-Andreas-

2015-06-11  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	PR rtl-optimization/66306
	* reload.c (find_reloads): Swap the match_dup info for
	commutative operands.

2015-06-11  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	PR rtl-optimization/66306
	* gcc.target/s390/pr66306.c: New test.

diff --git a/gcc/reload.c b/gcc/reload.c
index 2546c1b..48ad99c 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -3924,6 +3924,12 @@ find_reloads (rtx insn, int replace, int ind_levels, int live_known,
 	= *recog_data.operand_loc[commutative + 1];
       *recog_data.operand_loc[commutative + 1] = tem;
 
+      for (i = 0; i < recog_data.n_dups; i++)
+	if (recog_data.dup_num[i] == commutative
+	    || recog_data.dup_num[i] == commutative + 1)
+	  *recog_data.dup_loc[i]
+	    = recog_data.operand[(int) recog_data.dup_num[i]];
+
       for (i = 0; i < n_reloads; i++)
 	{
 	  if (rld[i].opnum == commutative)
diff --git a/gcc/testsuite/gcc.target/s390/pr66306.c b/gcc/testsuite/gcc.target/s390/pr66306.c
new file mode 100644
index 0000000..73903cb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr66306.c
@@ -0,0 +1,26 @@
+/* This caused an ICE on s390x due to a reload bug handling
+   commutative constraints.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+struct line_map
+{
+  unsigned start_location;
+  unsigned ordinary;
+};
+unsigned
+linemap_resolve_location (struct line_map **loc_map);
+
+unsigned
+linemap_position_for_loc_and_offset (unsigned h, unsigned loc)
+{
+  struct line_map *map = 0;
+  linemap_resolve_location (&map);
+
+  if (map->ordinary <= loc + map->start_location + map->ordinary)
+    __builtin_abort ();
+
+  if (h >= loc + map->start_location)
+    __builtin_abort ();
+}

Re: [PATCH 4.8] PR66306 Fix reload problem with commutative operands

[Ulrich Weigand]

Andreas Krebbel wrote:

> this fixes a reload problem with match_dup's on commutative operands.
> 
> Bootstrapped and regtested on x86-64, ppc64, and s390x.
> 
> Ok?
> 
> Bye,
> 
> -Andreas-
> 
> 2015-06-11  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
> 
> 	PR rtl-optimization/66306
> 	* reload.c (find_reloads): Swap the match_dup info for
> 	commutative operands.

This does indeed appear to be broken, and the fix looks good to me.

However, I'm not clear why this should be a 4.8 only patch ... the
same problem seems to be still there on mainline, right?

Patch is OK for mainline if it passes regression tests there.

Backports to release branches are fine with me if OK with the
release maintainers, after the change has been in mainline with
no problems reported for a bit.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 4.8] PR66306 Fix reload problem with commutative operands

[Andreas Krebbel]

On 06/16/2015 07:40 PM, Ulrich Weigand wrote:
> However, I'm not clear why this should be a 4.8 only patch ... the
> same problem seems to be still there on mainline, right?

Yes. But 4.8 appears to be the last where reload was really stable on these targets. When disabling
LRA on mainline x86_64 and s390x didn't even bootstrap (I'll have a look at the s390 issue). I've
got results for PPC64. There were no regressions (apart from two libgo tests which appear to be racy
and fail also without the patch from time to time: "runtime" and "time").

Bye,

-Andreas-

Re: [PATCH 4.8] PR66306 Fix reload problem with commutative operands

[Andreas Krebbel]

On 06/16/2015 07:40 PM, Ulrich Weigand wrote:
> Andreas Krebbel wrote:
> 
>> this fixes a reload problem with match_dup's on commutative operands.
>>
>> Bootstrapped and regtested on x86-64, ppc64, and s390x.
>>
>> Ok?
>>
>> Bye,
>>
>> -Andreas-
>>
>> 2015-06-11  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
>>
>> 	PR rtl-optimization/66306
>> 	* reload.c (find_reloads): Swap the match_dup info for
>> 	commutative operands.
> 
> This does indeed appear to be broken, and the fix looks good to me.
> 
> However, I'm not clear why this should be a 4.8 only patch ... the
> same problem seems to be still there on mainline, right?
> 
> Patch is OK for mainline if it passes regression tests there.

I've committed the patch after successful testing on PPC64 and s390x. I couldn't get reload working
on x86_64 quickly.

Bye,

-Andreas-