This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, i386]: FIx PR target/12329, x86: local function declared with attribute((regparm(3))) gets corrupted parent frame pointer
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: "Ross Ridge" <rridge at csclub dot uwaterloo dot ca>
- Cc: gcc-patches at gcc dot gnu dot org, "Ye, Joey" <joey dot ye at intel dot com>, "Guo, Xuepeng" <xuepeng dot guo at intel dot com>
- Date: Sat, 22 Mar 2008 08:33:32 -0700
- Subject: Re: [PATCH, i386]: FIx PR target/12329, x86: local function declared with attribute((regparm(3))) gets corrupted parent frame pointer
- References: <20080321204942.99B1573DAF@caffeine.csclub.uwaterloo.ca>
On Fri, Mar 21, 2008 at 1:49 PM, Ross Ridge <rridge@csclub.uwaterloo.ca> wrote:
> Uros Bizjak writes:
> >Attached patch fixes a couple of cases where %ecx register gets
> >corrupted wher regparm(3) is in effect. The patch limits the number of
> >regparms to 2 to preserve %ecx in these corner cases.
>
> Consider this case:
>
> int __attribute__((regparm(3))) foo(int a, int b, int c);
>
> int bar() {
> return foo(1, 2, 3)
> }
>
> int __attriubte__((regparm(3), force_align_arg_pointer))
> foo(int a, int b, int c) {
> return a + b + c;
> }
>
> Won't your patch result in foo() being called with three parameters in
> registers, despite foo() only expecting two parameters to be in registers?
>
> I don't think silently changing the calling convention is a good idea.
> If the regparm(3) attribute can't be honoured it should result in
> an error. The function might be called from assembly or code compiled
> from some future version of GCC that does honour regparm(3) in this case.
>
I am checking this patch into stack branch. It is safe since it only ignores
regparm(3) for nested function. The testcase works fine.
H.J.
2008-03-22 Uros Bizjak <ubizjak@gmail.com>
H.J. Lu <hongjiu.lu@intel.com>
PR target/12329
* config/i386/i386.c (ix86_function_regparm): Limit the number of
register passing arguments to 2 for nested functions.
testsuite/ChangeLog:
2008-03-21 Uros Bizjak <ubizjak@gmail.com>
PR target/12329
* gcc.target/i386/pr12329.c: New test.
--- gcc/config/i386/i386.c.reg 2008-03-21 06:09:48.000000000 -0700
+++ gcc/config/i386/i386.c 2008-03-22 08:23:17.000000000 -0700
@@ -3252,13 +3252,30 @@ ix86_function_regparm (const_tree type,
{
tree attr;
int regparm = ix86_regparm;
+ struct function *f;
if (TARGET_64BIT)
return regparm;
attr = lookup_attribute ("regparm", TYPE_ATTRIBUTES (type));
if (attr)
- return TREE_INT_CST_LOW (TREE_VALUE (TREE_VALUE (attr)));
+ {
+ regparm
+ = TREE_INT_CST_LOW (TREE_VALUE (TREE_VALUE (attr)));
+
+ if (decl && TREE_CODE (decl) == FUNCTION_DECL)
+ {
+ /* We can't use regparm(3) for nested functions as these use
+ static chain pointer in third argument. */
+ if (regparm == 3
+ && (decl_function_context (decl)
+ || ix86_force_align_arg_pointer)
+ && !DECL_NO_STATIC_CHAIN (decl))
+ regparm = 2;
+ }
+
+ return regparm;
+ }
if (lookup_attribute ("fastcall", TYPE_ATTRIBUTES (type)))
return 2;
@@ -3272,7 +3289,6 @@ ix86_function_regparm (const_tree type,
if (i && i->local)
{
int local_regparm, globals = 0, regno;
- struct function *f;
/* Make sure no regparm register is taken by a
fixed register variable. */
@@ -3307,7 +3323,7 @@ ix86_function_regparm (const_tree type,
local_regparm
= globals < local_regparm ? local_regparm - globals : 0;
- if (local_regparm > regparm)
+ if (regparm < local_regparm)
regparm = local_regparm;
}
}
--- gcc/gcc.target/i386/pr12329.c.reg 2008-03-22 08:21:57.000000000 -0700
+++ gcc/gcc.target/i386/pr12329.c 2008-03-22 08:21:57.000000000 -0700
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+/* { dg-require-effective-target ilp32 } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+int test_nested (int i)
+{
+ int __attribute__ ((__noinline__, __regparm__(3))) foo (int j, int k, int l)
+ {
+ return i + j + k + l;
+ }
+
+ return foo(i, i+1, i+2) * i;
+}
+
+int __attribute__ ((__noinline__, __regparm__(3), __force_align_arg_pointer__))
+test_realigned (int j, int k, int l)
+{
+ return j + k + l;
+}
+
+int main ()
+{
+ if (test_nested(10) != 430)
+ abort ();
+
+ if (test_realigned(10, 11, 12) != 33)
+ abort ();
+
+ return 0;
+}
+
--- gcc/testsuite/gcc.target/i386/pr12329.c.reg 2008-03-22 08:22:50.000000000 -0700
+++ gcc/testsuite/gcc.target/i386/pr12329.c 2008-03-22 08:22:50.000000000 -0700
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+/* { dg-require-effective-target ilp32 } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+int test_nested (int i)
+{
+ int __attribute__ ((__noinline__, __regparm__(3))) foo (int j, int k, int l)
+ {
+ return i + j + k + l;
+ }
+
+ return foo(i, i+1, i+2) * i;
+}
+
+int __attribute__ ((__noinline__, __regparm__(3), __force_align_arg_pointer__))
+test_realigned (int j, int k, int l)
+{
+ return j + k + l;
+}
+
+int main ()
+{
+ if (test_nested(10) != 430)
+ abort ();
+
+ if (test_realigned(10, 11, 12) != 33)
+ abort ();
+
+ return 0;
+}
+