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.

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);
+}

-- 
Alan Modra
Australia Development Lab, IBM

Reply via email to