diff mbox

[rs6000] Fix PR65171

Message ID 1424902129.3445.23.camel@gnopaine
State New
Headers show

Commit Message

Bill Schmidt Feb. 25, 2015, 10:08 p.m. UTC 
Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65171 identifies a bug when
compiling portions of the Boost library.  The problem occurs in the swap
analysis phase.  Any operand that is TImode or a subreg of TImode is
supposed to disable the swap optimization for the web of instructions
that includes it.  A logic error allowed this to behave properly for
V1TImode, but not for TImode.  This patch corrects that oversight.

The original test case is not suitable for use (far too large and
proprietary).  I am attempting to come up with a reduced test that shows
the problem but don't yet have one.  Since this is a pretty obvious fix
I would like to go forward with the patch, and add a test case at a
later time, if that's ok with you.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  Is this ok for trunk?

Thanks,
Bill


2015-02-25  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* config/rs6000/rs6000.c (rs6000_analyze_swaps): Ensure
	instructions with TImode operands are included in the analysis.

Comments

David Edelsohn Feb. 25, 2015, 11:31 p.m. UTC | #1 
On Wed, Feb 25, 2015 at 5:08 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65171 identifies a bug when
> compiling portions of the Boost library.  The problem occurs in the swap
> analysis phase.  Any operand that is TImode or a subreg of TImode is
> supposed to disable the swap optimization for the web of instructions
> that includes it.  A logic error allowed this to behave properly for
> V1TImode, but not for TImode.  This patch corrects that oversight.
>
> The original test case is not suitable for use (far too large and
> proprietary).  I am attempting to come up with a reduced test that shows
> the problem but don't yet have one.  Since this is a pretty obvious fix
> I would like to go forward with the patch, and add a test case at a
> later time, if that's ok with you.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?
>
> Thanks,
> Bill
>
>
> 2015-02-25  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * config/rs6000/rs6000.c (rs6000_analyze_swaps): Ensure
>         instructions with TImode operands are included in the analysis.

Okay.

I assume that support for PTImode is unnecessary because PTI is
limited to GPRs and never will appear in VRs, so would not be the root
of an appropriate web.

Yes, testcase at a later time, when it can be reduced.

Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 220916)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -34785,7 +34785,7 @@  rs6000_analyze_swaps (function *fun)
 		    mode = V4SImode;
 		}
 
-	      if (VECTOR_MODE_P (mode))
+	      if (VECTOR_MODE_P (mode) || mode == TImode)
 		{
 		  insn_entry[uid].is_relevant = 1;
 		  if (mode == TImode || mode == V1TImode)
@@ -34812,7 +34812,7 @@  rs6000_analyze_swaps (function *fun)
 		  && VECTOR_MODE_P (GET_MODE (SET_DEST (insn))))
 		mode = GET_MODE (SET_DEST (insn));
 
-	      if (VECTOR_MODE_P (mode))
+	      if (VECTOR_MODE_P (mode) || mode == TImode)
 		{
 		  insn_entry[uid].is_relevant = 1;
 		  if (mode == TImode || mode == V1TImode)