* [PATCH] SPU: Use FRAME_GROWS_DOWNWARD
@ 2008-10-21 15:07 Stefan Schulze Frielinghaus
2008-10-21 21:59 ` trevor_smigiel
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2008-10-21 15:07 UTC (permalink / raw)
To: gcc-patches; +Cc: trevor_smigiel, andrew_pinski
[-- Attachment #1: Type: text/plain, Size: 976 bytes --]
Hi,
to use the Stack Smashing Protector the frame needs to grow downwards.
Attached patch accomplishes that for the Cell B.E. SPU architecture.
Tested against regression suite without any difference in pass/fail
statistics (except the extra test passes for SSP)..
OK for mainline?
(See attached file: spu.patch)
Mit freundlichen Gruessen / Kind regards
Stefan Schulze Frielinghaus
IBM Systems & Technology Group, Systems Software Development
SW Linux on Cell BE Development & Evaluation
-------------------------------------------------------------------------------
IBM Deutschland
Schoenaicher Str. 220
71032 Boeblingen
Phone: ++49-(0)7031-16-2173
E-Mail: xxschulz@de.ibm.com
-------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
[-- Attachment #2: spu.patch --]
[-- Type: application/octet-stream, Size: 5931 bytes --]
2008-10-21 Stefan Schulze Frielinghaus <xxschulz@de.ibm.com>
* configure.ac: Remove skip dir libssp of SPU arch.
* configure: Re-generate.
* gcc/config/spu/spu.h (FRAME_GROWS_DOWNWARD)
(INITIAL_FRAME_POINTER_OFFSET): Define
FRAME_GROWS_DOWNWARD to 1 and remove unused
INITIAL_FRAME_POINTER_OFFSET.
* gcc/config/spu/spu.c (spu_initial_elimination_offset): Calculate new
offset if eliminating soft frame pointer.
* gcc/config/spu/spu.md (stack_protect_set, stack_protect_test)
(stack_protect_test_si): Add initial machine description
for Stack Smashing Protector
Index: configure
===================================================================
--- configure (revision 141105)
+++ configure (working copy)
@@ -2656,7 +2656,6 @@
sparc-*-solaris* | sparc64-*-solaris* | sparcv9-*-solaris*)
;;
spu-*-*)
- skipdirs="target-libssp"
;;
v810-*-*)
noconfigdirs="$noconfigdirs bfd binutils gas gcc gdb ld target-libstdc++-v3 opcodes target-libgloss ${libgcj}"
Index: gcc/config/spu/spu.c
===================================================================
--- gcc/config/spu/spu.c (revision 141105)
+++ gcc/config/spu/spu.c (working copy)
@@ -1759,8 +1759,8 @@
The stack frame looks like this:
+-------------+
| incoming |
- AP | args |
- +-------------+
+ | args |
+ AP -> +-------------+
| $lr save |
+-------------+
prev SP | back chain |
@@ -1770,10 +1770,10 @@
+-------------+
| ... |
| saved regs | spu_saved_regs_size() bytes
- +-------------+
+ FP -> +-------------+
| ... |
- FP | vars | get_frame_size() bytes
- +-------------+
+ | vars | get_frame_size() bytes
+ HFP -> +-------------+
| ... |
| outgoing |
| args | crtl->outgoing_args_size bytes
@@ -1781,8 +1781,8 @@
| $lr of next |
| frame |
+-------------+
- SP | back chain |
- +-------------+
+ | back chain |
+ SP -> +-------------+
*/
void
@@ -3671,15 +3671,16 @@
|| get_frame_size () || saved_regs_size)
sp_offset = STACK_POINTER_OFFSET;
if (from == FRAME_POINTER_REGNUM && to == STACK_POINTER_REGNUM)
- return (sp_offset + crtl->outgoing_args_size);
+ return get_frame_size () + crtl->outgoing_args_size + sp_offset;
else if (from == FRAME_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM)
- return 0;
+ return get_frame_size ();
else if (from == ARG_POINTER_REGNUM && to == STACK_POINTER_REGNUM)
return sp_offset + crtl->outgoing_args_size
+ get_frame_size () + saved_regs_size + STACK_POINTER_OFFSET;
else if (from == ARG_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM)
return get_frame_size () + saved_regs_size + sp_offset;
- return 0;
+ else
+ gcc_unreachable ();
}
rtx
Index: gcc/config/spu/spu.h
===================================================================
--- gcc/config/spu/spu.h (revision 141105)
+++ gcc/config/spu/spu.h (working copy)
@@ -251,6 +251,8 @@
#define STACK_GROWS_DOWNWARD
+#define FRAME_GROWS_DOWNWARD 1
+
#define STARTING_FRAME_OFFSET (0)
#define STACK_POINTER_OFFSET 32
@@ -312,8 +314,6 @@
#define FRAME_POINTER_REQUIRED 0
-#define INITIAL_FRAME_POINTER_OFFSET(DEPTH) ((DEPTH) = 0)
-
#define ELIMINABLE_REGS \
{{ARG_POINTER_REGNUM, STACK_POINTER_REGNUM}, \
{ARG_POINTER_REGNUM, HARD_FRAME_POINTER_REGNUM}, \
Index: gcc/config/spu/spu.md
===================================================================
--- gcc/config/spu/spu.md (revision 141105)
+++ gcc/config/spu/spu.md (working copy)
@@ -155,6 +155,8 @@
(UNSPEC_DFTSV 51)
(UNSPEC_FLOAT_EXTEND 52)
(UNSPEC_FLOAT_TRUNCATE 53)
+ (UNSPEC_SP_SET 54)
+ (UNSPEC_SP_TEST 55)
])
(include "predicates.md")
@@ -5188,4 +5190,51 @@
DONE;
}")
+(define_insn "stack_protect_set"
+ [(set (match_operand:SI 0 "memory_operand" "=m")
+ (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] UNSPEC_SP_SET))
+ (set (match_scratch:SI 2 "=&r") (const_int 0))]
+ ""
+ "lqr\t%2,%1\;stqd\t%2,%0\;xor\t%2,%2,%2"
+ [(set_attr "length" "12")
+ (set_attr "type" "multi0")]
+)
+(define_expand "stack_protect_test"
+ [(match_operand 0 "memory_operand" "")
+ (match_operand 1 "memory_operand" "")
+ (match_operand 2 "" "")]
+ ""
+{
+ rtx compare_result;
+ rtx bcomp, loc_ref;
+
+ compare_result = gen_reg_rtx (SImode);
+
+ emit_insn (gen_stack_protect_test_si (compare_result,
+ operands[0],
+ operands[1]));
+
+ bcomp = gen_rtx_NE (SImode, compare_result, const0_rtx);
+
+ loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands[2]);
+
+ emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+ gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+ loc_ref, pc_rtx)));
+
+ DONE;
+})
+
+(define_insn "stack_protect_test_si"
+ [(set (match_operand:SI 0 "register_operand" "+r")
+ (unspec:SI [(match_operand:SI 1 "memory_operand" "m")
+ (match_operand:SI 2 "memory_operand" "m")]
+ UNSPEC_SP_TEST))
+ (set (match_scratch:SI 3 "=&r") (const_int 0))]
+ ""
+ "lqd\t%0,%1\;lqr\t%3,%2\;ceq\t%0,%0,%3\;xor\t%3,%3,%3"
+ [(set_attr "length" "16")
+ (set_attr "type" "multi0")]
+)
+
Index: configure.ac
===================================================================
--- configure.ac (revision 141105)
+++ configure.ac (working copy)
@@ -892,7 +892,6 @@
sparc-*-solaris* | sparc64-*-solaris* | sparcv9-*-solaris*)
;;
spu-*-*)
- skipdirs="target-libssp"
;;
v810-*-*)
noconfigdirs="$noconfigdirs bfd binutils gas gcc gdb ld target-libstdc++-v3 opcodes target-libgloss ${libgcj}"
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] SPU: Use FRAME_GROWS_DOWNWARD
2008-10-21 15:07 [PATCH] SPU: Use FRAME_GROWS_DOWNWARD Stefan Schulze Frielinghaus
@ 2008-10-21 21:59 ` trevor_smigiel
[not found] ` <OF7C80CD55.C050186C-ONC12574EB.00650AC9-C12574EB.00679933@de.ibm.com>
0 siblings, 1 reply; 6+ messages in thread
From: trevor_smigiel @ 2008-10-21 21:59 UTC (permalink / raw)
To: Stefan Schulze Frielinghaus; +Cc: gcc-patches, andrew_pinski
Stefan,
In the "stack_protect_set" and "stack_protect_test_si" patterns, the
type attribute should be multi1. This indicates that the first
instruction is in the odd pipeline, which allows slightly better
scheduling of these patterns.
In "stack_protect_test_si", shouldn't the "+r" of the first operand be
"=&r"? Also, please change "register_operand" to "spu_reg_operand".
Based on the code, it is not necessary now, but it protects us from
possible issues in the future.
I'm a little suspicious of the memory predicates, constraints, and the
hard coded load/store instructions. How do we know that the memory rtl
will always match the hard coded instructions?
How did you compare the test suite results? Did you use
contrib/compare_tests? Or just compare the numbers/statistics printed
at the end? (Use compare_tests.)
Trevor
* Stefan Schulze Frielinghaus <XXSCHULZ@de.ibm.com> [2008-10-21 07:00]:
>
> Hi,
>
> to use the Stack Smashing Protector the frame needs to grow downwards.
> Attached patch accomplishes that for the Cell B.E. SPU architecture.
>
> Tested against regression suite without any difference in pass/fail
> statistics (except the extra test passes for SSP)..
>
> OK for mainline?
> (See attached file: spu.patch)
> Mit freundlichen Gruessen / Kind regards
>
> Stefan Schulze Frielinghaus
>
> IBM Systems & Technology Group, Systems Software Development
> SW Linux on Cell BE Development & Evaluation
> -------------------------------------------------------------------------------
>
> IBM Deutschland
> Schoenaicher Str. 220
> 71032 Boeblingen
> Phone: ++49-(0)7031-16-2173
> E-Mail: xxschulz@de.ibm.com
> -------------------------------------------------------------------------------
>
> IBM Deutschland Research & Development GmbH
> Vorsitzender des Aufsichtsrats: Martin Jetter
> Geschaeftsfuehrung: Herbert Kircher
> Sitz der Gesellschaft: Boeblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SPU: Use FRAME_GROWS_DOWNWARD
@ 2008-10-24 8:30 Stefan Schulze Frielinghaus
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2008-10-24 8:30 UTC (permalink / raw)
To: trevor_smigiel; +Cc: andrew_pinski, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]
gcc-patches-owner@gcc.gnu.org wrote on 10/21/2008 09:42:56 PM:
> <trevor_smigiel@playstation.sony.com>
> Sent by: gcc-patches-owner@gcc.gnu.org
[...]
> In the "stack_protect_set" and "stack_protect_test_si" patterns, the
> type attribute should be multi1. This indicates that the first
> instruction is in the odd pipeline, which allows slightly better
> scheduling of these patterns.
Sure, load/store always computes in pipe 1. Changed
>
> In "stack_protect_test_si", shouldn't the "+r" of the first operand be
> "=&r"? Also, please change "register_operand" to "spu_reg_operand".
> Based on the code, it is not necessary now, but it protects us from
> possible issues in the future.
Changed.
>
> I'm a little suspicious of the memory predicates, constraints, and the
> hard coded load/store instructions. How do we know that the memory rtl
> will always match the hard coded instructions?
Right. Changed to %p and spu_mem_operand.
>
> How did you compare the test suite results? Did you use
> contrib/compare_tests? Or just compare the numbers/statistics printed
> at the end? (Use compare_tests.)
compare_tests result:
New tests that PASS::
gcc.dg/pr34225.c (test for excess errors)
This test passes because it needs -fstack-protector.
New patch tested with same results as previous patch.
(See attached file: spu.patch)
Mit freundlichen Gruessen / Kind regards
Stefan Schulze Frielinghaus
IBM Systems & Technology Group, Systems Software Development
SW Linux on Cell BE Development & Evaluation
-------------------------------------------------------------------------------
IBM Deutschland
Schoenaicher Str. 220
71032 Boeblingen
Phone: ++49-(0)7031-16-2173
E-Mail: xxschulz@de.ibm.com
-------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
[-- Attachment #2: spu.patch --]
[-- Type: application/octet-stream, Size: 5945 bytes --]
2008-10-23 Stefan Schulze Frielinghaus <xxschulz@de.ibm.com>
* configure.ac: Remove skip dir libssp of SPU arch.
* configure: Re-generate.
* gcc/config/spu/spu.h (FRAME_GROWS_DOWNWARD)
(INITIAL_FRAME_POINTER_OFFSET): Define
FRAME_GROWS_DOWNWARD to 1 and remove unused
INITIAL_FRAME_POINTER_OFFSET.
* gcc/config/spu/spu.c (spu_initial_elimination_offset): Calculate new
offset if eliminating soft frame pointer.
* gcc/config/spu/spu.md (stack_protect_set, stack_protect_test)
(stack_protect_test_si): Add initial machine description
for Stack Smashing Protector
Index: configure
===================================================================
--- configure (revision 141289)
+++ configure (working copy)
@@ -2656,7 +2656,6 @@
sparc-*-solaris* | sparc64-*-solaris* | sparcv9-*-solaris*)
;;
spu-*-*)
- skipdirs="target-libssp"
;;
v810-*-*)
noconfigdirs="$noconfigdirs bfd binutils gas gcc gdb ld target-libstdc++-v3 opcodes target-libgloss ${libgcj}"
Index: gcc/config/spu/spu.c
===================================================================
--- gcc/config/spu/spu.c (revision 141289)
+++ gcc/config/spu/spu.c (working copy)
@@ -1759,8 +1759,8 @@
The stack frame looks like this:
+-------------+
| incoming |
- AP | args |
- +-------------+
+ | args |
+ AP -> +-------------+
| $lr save |
+-------------+
prev SP | back chain |
@@ -1770,10 +1770,10 @@
+-------------+
| ... |
| saved regs | spu_saved_regs_size() bytes
- +-------------+
+ FP -> +-------------+
| ... |
- FP | vars | get_frame_size() bytes
- +-------------+
+ | vars | get_frame_size() bytes
+ HFP -> +-------------+
| ... |
| outgoing |
| args | crtl->outgoing_args_size bytes
@@ -1781,8 +1781,8 @@
| $lr of next |
| frame |
+-------------+
- SP | back chain |
- +-------------+
+ | back chain |
+ SP -> +-------------+
*/
void
@@ -3671,15 +3671,16 @@
|| get_frame_size () || saved_regs_size)
sp_offset = STACK_POINTER_OFFSET;
if (from == FRAME_POINTER_REGNUM && to == STACK_POINTER_REGNUM)
- return (sp_offset + crtl->outgoing_args_size);
+ return get_frame_size () + crtl->outgoing_args_size + sp_offset;
else if (from == FRAME_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM)
- return 0;
+ return get_frame_size ();
else if (from == ARG_POINTER_REGNUM && to == STACK_POINTER_REGNUM)
return sp_offset + crtl->outgoing_args_size
+ get_frame_size () + saved_regs_size + STACK_POINTER_OFFSET;
else if (from == ARG_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM)
return get_frame_size () + saved_regs_size + sp_offset;
- return 0;
+ else
+ gcc_unreachable ();
}
rtx
Index: gcc/config/spu/spu.h
===================================================================
--- gcc/config/spu/spu.h (revision 141289)
+++ gcc/config/spu/spu.h (working copy)
@@ -251,6 +251,8 @@
#define STACK_GROWS_DOWNWARD
+#define FRAME_GROWS_DOWNWARD 1
+
#define STARTING_FRAME_OFFSET (0)
#define STACK_POINTER_OFFSET 32
@@ -312,8 +314,6 @@
#define FRAME_POINTER_REQUIRED 0
-#define INITIAL_FRAME_POINTER_OFFSET(DEPTH) ((DEPTH) = 0)
-
#define ELIMINABLE_REGS \
{{ARG_POINTER_REGNUM, STACK_POINTER_REGNUM}, \
{ARG_POINTER_REGNUM, HARD_FRAME_POINTER_REGNUM}, \
Index: gcc/config/spu/spu.md
===================================================================
--- gcc/config/spu/spu.md (revision 141289)
+++ gcc/config/spu/spu.md (working copy)
@@ -155,6 +155,8 @@
(UNSPEC_DFTSV 51)
(UNSPEC_FLOAT_EXTEND 52)
(UNSPEC_FLOAT_TRUNCATE 53)
+ (UNSPEC_SP_SET 54)
+ (UNSPEC_SP_TEST 55)
])
(include "predicates.md")
@@ -5188,4 +5190,51 @@
DONE;
}")
+(define_insn "stack_protect_set"
+ [(set (match_operand:SI 0 "spu_mem_operand" "=m")
+ (unspec:SI [(match_operand:SI 1 "spu_mem_operand" "m")] UNSPEC_SP_SET))
+ (set (match_scratch:SI 2 "=&r") (const_int 0))]
+ ""
+ "lq%p1\t%2,%1\;stq%p0\t%2,%0\;xor\t%2,%2,%2"
+ [(set_attr "length" "12")
+ (set_attr "type" "multi1")]
+)
+(define_expand "stack_protect_test"
+ [(match_operand 0 "spu_mem_operand" "")
+ (match_operand 1 "spu_mem_operand" "")
+ (match_operand 2 "" "")]
+ ""
+{
+ rtx compare_result;
+ rtx bcomp, loc_ref;
+
+ compare_result = gen_reg_rtx (SImode);
+
+ emit_insn (gen_stack_protect_test_si (compare_result,
+ operands[0],
+ operands[1]));
+
+ bcomp = gen_rtx_NE (SImode, compare_result, const0_rtx);
+
+ loc_ref = gen_rtx_LABEL_REF (VOIDmode, operands[2]);
+
+ emit_jump_insn (gen_rtx_SET (VOIDmode, pc_rtx,
+ gen_rtx_IF_THEN_ELSE (VOIDmode, bcomp,
+ loc_ref, pc_rtx)));
+
+ DONE;
+})
+
+(define_insn "stack_protect_test_si"
+ [(set (match_operand:SI 0 "spu_reg_operand" "=&r")
+ (unspec:SI [(match_operand:SI 1 "spu_mem_operand" "m")
+ (match_operand:SI 2 "spu_mem_operand" "m")]
+ UNSPEC_SP_TEST))
+ (set (match_scratch:SI 3 "=&r") (const_int 0))]
+ ""
+ "lq%p1\t%0,%1\;lq%p2\t%3,%2\;ceq\t%0,%0,%3\;xor\t%3,%3,%3"
+ [(set_attr "length" "16")
+ (set_attr "type" "multi1")]
+)
+
Index: configure.ac
===================================================================
--- configure.ac (revision 141289)
+++ configure.ac (working copy)
@@ -892,7 +892,6 @@
sparc-*-solaris* | sparc64-*-solaris* | sparcv9-*-solaris*)
;;
spu-*-*)
- skipdirs="target-libssp"
;;
v810-*-*)
noconfigdirs="$noconfigdirs bfd binutils gas gcc gdb ld target-libstdc++-v3 opcodes target-libgloss ${libgcj}"
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-24 6:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-21 15:07 [PATCH] SPU: Use FRAME_GROWS_DOWNWARD Stefan Schulze Frielinghaus
2008-10-21 21:59 ` trevor_smigiel
[not found] ` <OF7C80CD55.C050186C-ONC12574EB.00650AC9-C12574EB.00679933@de.ibm.com>
2008-10-23 22:00 ` trevor_smigiel
2008-10-23 22:19 ` Ben Elliston
2008-10-24 8:46 ` Stefan Schulze Frielinghaus
2008-10-24 8:30 Stefan Schulze Frielinghaus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).