diff mbox

[rs6000] Fix PR78695

Message ID b6c93896-3618-dec5-f1e9-0f03a7bc6ee6@linux.vnet.ibm.com
State New
Headers show

Commit Message

Bill Schmidt Dec. 11, 2016, 9:31 p.m. UTC
Hi Segher,

On 12/11/16 2:00 PM, Segher Boessenkool wrote:
> Maybe this should use DF_REF_IS_ARTIFICIAL?  Or if that doesn't work,
> DF_REF_INSN_INFO?
>
OK, currently regstrapping the following, which also fixes the problem with
a non-bootstrap compiler.  Is this ok for trunk if it succeeds?

Thanks,
Bill


[gcc]

2016-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/78695
	* config/rs6000/rs6000.c (find_alignment_op): Discard from
	consideration any artificial definition.

[gcc/testsuite]

2016-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/78695
	* gcc.target/powerpc/swaps-stack-protector.c: New test.

Comments

Bill Schmidt Dec. 11, 2016, 11:04 p.m. UTC | #1
Hi Segher, 

This indeed bootstrapped on powerpc64le-unknown-linux-gnu with no
regressions.  Ok for trunk?

Thanks for the review!

Bill

On 12/11/16 3:31 PM, Bill Schmidt wrote:
> Hi Segher,
>
> On 12/11/16 2:00 PM, Segher Boessenkool wrote:
>> Maybe this should use DF_REF_IS_ARTIFICIAL?  Or if that doesn't work,
>> DF_REF_INSN_INFO?
>>
> OK, currently regstrapping the following, which also fixes the problem with
> a non-bootstrap compiler.  Is this ok for trunk if it succeeds?
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2016-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
> 	PR target/78695
> 	* config/rs6000/rs6000.c (find_alignment_op): Discard from
> 	consideration any artificial definition.
>
> [gcc/testsuite]
>
> 2016-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
> 	PR target/78695
> 	* gcc.target/powerpc/swaps-stack-protector.c: New test.
>
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 243506)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -41433,6 +41433,12 @@ find_alignment_op (rtx_insn *insn, rtx base_reg)
>        if (!base_def_link || base_def_link->next)
>  	break;
>
> +      /* With stack-protector code enabled, and possibly in other
> +	 circumstances, there may not be an associated insn for 
> +	 the def.  */
> +      if (DF_REF_CLASS (base_def_link->ref) == DF_REF_ARTIFICIAL)
> +	break;
> +
>        rtx_insn *and_insn = DF_REF_INSN (base_def_link->ref);
>        and_operation = alignment_mask (and_insn);
>        if (and_operation != 0)
> Index: gcc/testsuite/gcc.target/powerpc/swaps-stack-protector.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/swaps-stack-protector.c	(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/swaps-stack-protector.c	(working copy)
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fstack-protector -O3" } */
> +
> +/* PR78695: This code used to ICE in rs6000.c:find_alignment_op because
> +   the stack protector address definition isn't associated with an insn.  */
> +
> +void *a();
> +long b() {
> +  char c[1];
> +  char *d = a(), *e = c;
> +  long f = e ? b(e) : 0;
> +  if (f > 54)
> +    f = 1;
> +  while (f--)
> +    *d++ = *e++;
> +}
>
Segher Boessenkool Dec. 11, 2016, 11:29 p.m. UTC | #2
On Sun, Dec 11, 2016 at 03:31:35PM -0600, Bill Schmidt wrote:
> On 12/11/16 2:00 PM, Segher Boessenkool wrote:
> > Maybe this should use DF_REF_IS_ARTIFICIAL?  Or if that doesn't work,
> > DF_REF_INSN_INFO?
> >
> OK, currently regstrapping the following, which also fixes the problem with
> a non-bootstrap compiler.  Is this ok for trunk if it succeeds?

> --- gcc/config/rs6000/rs6000.c	(revision 243506)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -41433,6 +41433,12 @@ find_alignment_op (rtx_insn *insn, rtx base_reg)
>        if (!base_def_link || base_def_link->next)
>  	break;
>  
> +      /* With stack-protector code enabled, and possibly in other
> +	 circumstances, there may not be an associated insn for 
> +	 the def.  */
> +      if (DF_REF_CLASS (base_def_link->ref) == DF_REF_ARTIFICIAL)
> +	break;

  if (DF_REF_IS_ARTIFICIAL (base_def_link->ref))

Okay with that (no need to retest, just see if it compiles ;-) )

Thanks,


Segher
Bill Schmidt Dec. 11, 2016, 11:32 p.m. UTC | #3
Ah, I misread your comment and went hunting for DF_REF_ARTIFICIAL.  Sorry!  Will fix.

Thanks again,
Bill

> On Dec 11, 2016, at 5:29 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Sun, Dec 11, 2016 at 03:31:35PM -0600, Bill Schmidt wrote:
>> On 12/11/16 2:00 PM, Segher Boessenkool wrote:
>>> Maybe this should use DF_REF_IS_ARTIFICIAL?  Or if that doesn't work,
>>> DF_REF_INSN_INFO?
>>> 
>> OK, currently regstrapping the following, which also fixes the problem with
>> a non-bootstrap compiler.  Is this ok for trunk if it succeeds?
> 
>> --- gcc/config/rs6000/rs6000.c	(revision 243506)
>> +++ gcc/config/rs6000/rs6000.c	(working copy)
>> @@ -41433,6 +41433,12 @@ find_alignment_op (rtx_insn *insn, rtx base_reg)
>>       if (!base_def_link || base_def_link->next)
>> 	break;
>> 
>> +      /* With stack-protector code enabled, and possibly in other
>> +	 circumstances, there may not be an associated insn for 
>> +	 the def.  */
>> +      if (DF_REF_CLASS (base_def_link->ref) == DF_REF_ARTIFICIAL)
>> +	break;
> 
>  if (DF_REF_IS_ARTIFICIAL (base_def_link->ref))
> 
> Okay with that (no need to retest, just see if it compiles ;-) )
> 
> Thanks,
> 
> 
> Segher
>
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 243506)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -41433,6 +41433,12 @@  find_alignment_op (rtx_insn *insn, rtx base_reg)
       if (!base_def_link || base_def_link->next)
 	break;
 
+      /* With stack-protector code enabled, and possibly in other
+	 circumstances, there may not be an associated insn for 
+	 the def.  */
+      if (DF_REF_CLASS (base_def_link->ref) == DF_REF_ARTIFICIAL)
+	break;
+
       rtx_insn *and_insn = DF_REF_INSN (base_def_link->ref);
       and_operation = alignment_mask (and_insn);
       if (and_operation != 0)
Index: gcc/testsuite/gcc.target/powerpc/swaps-stack-protector.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/swaps-stack-protector.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/swaps-stack-protector.c	(working copy)
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fstack-protector -O3" } */
+
+/* PR78695: This code used to ICE in rs6000.c:find_alignment_op because
+   the stack protector address definition isn't associated with an insn.  */
+
+void *a();
+long b() {
+  char c[1];
+  char *d = a(), *e = c;
+  long f = e ? b(e) : 0;
+  if (f > 54)
+    f = 1;
+  while (f--)
+    *d++ = *e++;
+}