diff mbox

[RS6000] fix for RTL cprop vs. fixed hard regs

Message ID 20150130065713.GF14796@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Jan. 30, 2015, 6:57 a.m. UTC 
On Fri, Jan 16, 2015 at 08:12:27PM +1030, Alan Modra wrote:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-December/123776.html
> shows gcc-5 miscompiling a powerpc64 linux kernel.  The executive
> summary is that the rs6000 backend has a bug in its RTL description of
> indirect calls.  We specify a parallel containing both the actual call
> and an action that happens after the call, the restore of r2.  The
> restore is simply a memory load:
>             (set (reg:DI 2 2)
>                 (mem/v/c:DI (plus:DI (reg/f:DI 1 1)
>                         (const_int 40 [0x28])) [0  S8 A8]))
> This leads to cprop concluding that it is valid to replace the
> reference to r1 with another register having the same value before the
> call.  Unfortunately, sometimes a call-clobbered register is chosen.

This is the rs6000 backend fix.  Bootstrapped etc. powerpc64-linux.
OK to apply?

gcc/
	* config/rs6000/rs6000.c (rs6000_call_aix): Use unspec rather
	than mem for toc_restore.
	* config/rs6000/rs6000.md (UNSPEC_TOCSLOT): Define.
	(call_indirect_aix, call_value_indirect_aix): Adjust to suit.
	(call_indirect_elfv2, call_value_indirect_elfv2): Likewise.
gcc/testsuite/
	* gcc.target/powerpc/cprophard.c: New.

Comments

David Edelsohn Feb. 2, 2015, 1:34 a.m. UTC | #1 
On Fri, Jan 30, 2015 at 1:57 AM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Jan 16, 2015 at 08:12:27PM +1030, Alan Modra wrote:
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-December/123776.html
>> shows gcc-5 miscompiling a powerpc64 linux kernel.  The executive
>> summary is that the rs6000 backend has a bug in its RTL description of
>> indirect calls.  We specify a parallel containing both the actual call
>> and an action that happens after the call, the restore of r2.  The
>> restore is simply a memory load:
>>             (set (reg:DI 2 2)
>>                 (mem/v/c:DI (plus:DI (reg/f:DI 1 1)
>>                         (const_int 40 [0x28])) [0  S8 A8]))
>> This leads to cprop concluding that it is valid to replace the
>> reference to r1 with another register having the same value before the
>> call.  Unfortunately, sometimes a call-clobbered register is chosen.
>
> This is the rs6000 backend fix.  Bootstrapped etc. powerpc64-linux.
> OK to apply?
>
> gcc/
>         * config/rs6000/rs6000.c (rs6000_call_aix): Use unspec rather
>         than mem for toc_restore.
>         * config/rs6000/rs6000.md (UNSPEC_TOCSLOT): Define.

Please insert UNSPEC_TOCSLOT in the UNSPEC list after UNSPEC_TOCPTR
and UNSPEC_TOC and add  a short comment like the other two.

>         (call_indirect_aix, call_value_indirect_aix): Adjust to suit.
>         (call_indirect_elfv2, call_value_indirect_elfv2): Likewise.
> gcc/testsuite/
>         * gcc.target/powerpc/cprophard.c: New.

Okay with that change.

Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 220025)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -32934,7 +32934,10 @@  rs6000_call_aix (rtx value, rtx func_desc, rtx fla
       rtx stack_toc_mem = gen_frame_mem (Pmode,
 					 gen_rtx_PLUS (Pmode, stack_ptr,
 						       stack_toc_offset));
-      toc_restore = gen_rtx_SET (VOIDmode, toc_reg, stack_toc_mem);
+      rtx stack_toc_unspec = gen_rtx_UNSPEC (Pmode,
+					     gen_rtvec (1, stack_toc_offset),
+					     UNSPEC_TOCSLOT);
+      toc_restore = gen_rtx_SET (VOIDmode, toc_reg, stack_toc_unspec);
 
       /* Can we optimize saving the TOC in the prologue or
 	 do we need to do it at every call?  */
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 220025)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -138,6 +138,7 @@ 
    UNSPEC_PACK_128BIT
    UNSPEC_LSQ
    UNSPEC_FUSION_GPR
+   UNSPEC_TOCSLOT
   ])
 
 ;;
@@ -11348,16 +11349,16 @@ 
 ;; Call to indirect functions with the AIX abi using a 3 word descriptor.
 ;; Operand0 is the addresss of the function to call
 ;; Operand2 is the location in the function descriptor to load r2 from
-;; Operand3 is the stack location to hold the current TOC pointer
+;; Operand3 is the offset of the stack location holding the current TOC pointer
 
 (define_insn "*call_indirect_aix<mode>"
   [(call (mem:SI (match_operand:P 0 "register_operand" "c,*l"))
 	 (match_operand 1 "" "g,g"))
    (use (match_operand:P 2 "memory_operand" "<ptrm>,<ptrm>"))
-   (set (reg:P TOC_REGNUM) (match_operand:P 3 "memory_operand" "<ptrm>,<ptrm>"))
+   (set (reg:P TOC_REGNUM) (unspec [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_AIX"
-  "<ptrload> 2,%2\;b%T0l\;<ptrload> 2,%3"
+  "<ptrload> 2,%2\;b%T0l\;<ptrload> 2,%3(1)"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "12")])
 
@@ -11366,24 +11367,24 @@ 
 	(call (mem:SI (match_operand:P 1 "register_operand" "c,*l"))
 	      (match_operand 2 "" "g,g")))
    (use (match_operand:P 3 "memory_operand" "<ptrm>,<ptrm>"))
-   (set (reg:P TOC_REGNUM) (match_operand:P 4 "memory_operand" "<ptrm>,<ptrm>"))
+   (set (reg:P TOC_REGNUM) (unspec [(match_operand:P 4 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_AIX"
-  "<ptrload> 2,%3\;b%T1l\;<ptrload> 2,%4"
+  "<ptrload> 2,%3\;b%T1l\;<ptrload> 2,%4(1)"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "12")])
 
 ;; Call to indirect functions with the ELFv2 ABI.
 ;; Operand0 is the addresss of the function to call
-;; Operand2 is the stack location to hold the current TOC pointer
+;; Operand2 is the offset of the stack location holding the current TOC pointer
 
 (define_insn "*call_indirect_elfv2<mode>"
   [(call (mem:SI (match_operand:P 0 "register_operand" "c,*l"))
 	 (match_operand 1 "" "g,g"))
-   (set (reg:P TOC_REGNUM) (match_operand:P 2 "memory_operand" "<ptrm>,<ptrm>"))
+   (set (reg:P TOC_REGNUM) (unspec [(match_operand:P 2 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_ELFv2"
-  "b%T0l\;<ptrload> 2,%2"
+  "b%T0l\;<ptrload> 2,%2(1)"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "8")])
 
@@ -11391,10 +11392,10 @@ 
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:P 1 "register_operand" "c,*l"))
 	      (match_operand 2 "" "g,g")))
-   (set (reg:P TOC_REGNUM) (match_operand:P 3 "memory_operand" "<ptrm>,<ptrm>"))
+   (set (reg:P TOC_REGNUM) (unspec [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
    (clobber (reg:P LR_REGNO))]
   "DEFAULT_ABI == ABI_ELFv2"
-  "b%T1l\;<ptrload> 2,%3"
+  "b%T1l\;<ptrload> 2,%3(1)"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "8")])
 
Index: gcc/testsuite/gcc.target/powerpc/cprophard.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/cprophard.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/cprophard.c	(working copy)
@@ -0,0 +1,13 @@ 
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler {ld 2,(24|40)\(1\)} } } */
+
+/* From a linux kernel mis-compile of net/core/skbuff.c.  */
+register unsigned long current_r1 asm ("r1");
+
+void f (unsigned int n, void (*fun) (unsigned long))
+{
+  while (n--)
+    (*fun) (current_r1 & -0x1000);
+}