| Message ID | 20140205123722.GB12671@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 5 Feb 2014, Jakub Jelinek wrote: > Hi! > > Using !!optimize to determine if we should switch local ABI to regparm > convention isn't compatible with optimize attribute, as !!optimize is > whether the current function is being optimized, but for the ABI decisions > we actually need the caller and callee to agree on the calling convention. > > Fixed by looking at callee's optimize settings all the time. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Seeing a 2nd variant of such code I'd rather have an abstraction for this ... optimize_fn ()? opt_for_fn (fn, OPT_...)? Richard. > 2014-02-05 Jakub Jelinek <jakub@redhat.com> > > PR target/60062 > * config/i386/i386.c (ix86_function_regparm): Use optimize > from the callee instead of current function's optimize > to determine if local regparm convention should be used. > > * gcc.c-torture/execute/pr60062.c: New test. > * gcc.c-torture/execute/pr60072.c: New test. > > --- gcc/config/i386/i386.c.jj 2014-02-04 01:36:00.000000000 +0100 > +++ gcc/config/i386/i386.c 2014-02-05 09:09:29.603827877 +0100 > @@ -5608,11 +5608,22 @@ ix86_function_regparm (const_tree type, > /* Use register calling convention for local functions when possible. */ > if (decl > && TREE_CODE (decl) == FUNCTION_DECL > - && optimize > && !(profile_flag && !flag_fentry)) > { > /* FIXME: remove this CONST_CAST when cgraph.[ch] is constified. */ > struct cgraph_local_info *i = cgraph_local_info (CONST_CAST_TREE (decl)); > + > + /* Caller and callee must agree on the calling convention, so > + checking here just optimize means that with > + __attribute__((optimize (...))) caller could use regparm convention > + and callee not, or vice versa. Instead look at whether the callee > + is optimized or not. */ > + tree opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl); > + if (opts == NULL_TREE) > + opts = optimization_default_node; > + if (!TREE_OPTIMIZATION (opts)->x_optimize) > + i = NULL; > + > if (i && i->local && i->can_change_signature) > { > int local_regparm, globals = 0, regno; > --- gcc/testsuite/gcc.c-torture/execute/pr60062.c.jj 2014-02-05 09:02:13.703085235 +0100 > +++ gcc/testsuite/gcc.c-torture/execute/pr60062.c 2014-02-05 09:01:54.000000000 +0100 > @@ -0,0 +1,25 @@ > +/* PR target/60062 */ > + > +int a; > + > +static void > +foo (const char *p1, int p2) > +{ > + if (__builtin_strcmp (p1, "hello") != 0) > + __builtin_abort (); > +} > + > +static void > +bar (const char *p1) > +{ > + if (__builtin_strcmp (p1, "hello") != 0) > + __builtin_abort (); > +} > + > +__attribute__((optimize (0))) int > +main () > +{ > + foo ("hello", a); > + bar ("hello"); > + return 0; > +} > --- gcc/testsuite/gcc.c-torture/execute/pr60072.c.jj 2013-08-25 18:20:55.717911035 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr60072.c 2014-02-05 11:33:07.501748426 +0100 > @@ -0,0 +1,16 @@ > +/* PR target/60072 */ > + > +int c = 1; > + > +__attribute__ ((optimize (1))) > +static int *foo (int *p) > +{ > + return p; > +} > + > +int > +main () > +{ > + *foo (&c) = 2; > + return c - 2; > +} > > Jakub > >
> On Wed, 5 Feb 2014, Jakub Jelinek wrote: > > > Hi! > > > > Using !!optimize to determine if we should switch local ABI to regparm > > convention isn't compatible with optimize attribute, as !!optimize is > > whether the current function is being optimized, but for the ABI decisions > > we actually need the caller and callee to agree on the calling convention. > > > > Fixed by looking at callee's optimize settings all the time. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > Seeing a 2nd variant of such code I'd rather have an abstraction > for this ... optimize_fn ()? opt_for_fn (fn, OPT_...)? Propbable opt_for_fn. I wanted to implement SSE passing conventions for local functions but didn't get to it since I was concerned about -fno-sse being passed to teh callee by target attribute. Having API to check those would be nice. Honza
--- gcc/config/i386/i386.c.jj 2014-02-04 01:36:00.000000000 +0100 +++ gcc/config/i386/i386.c 2014-02-05 09:09:29.603827877 +0100 @@ -5608,11 +5608,22 @@ ix86_function_regparm (const_tree type, /* Use register calling convention for local functions when possible. */ if (decl && TREE_CODE (decl) == FUNCTION_DECL - && optimize && !(profile_flag && !flag_fentry)) { /* FIXME: remove this CONST_CAST when cgraph.[ch] is constified. */ struct cgraph_local_info *i = cgraph_local_info (CONST_CAST_TREE (decl)); + + /* Caller and callee must agree on the calling convention, so + checking here just optimize means that with + __attribute__((optimize (...))) caller could use regparm convention + and callee not, or vice versa. Instead look at whether the callee + is optimized or not. */ + tree opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl); + if (opts == NULL_TREE) + opts = optimization_default_node; + if (!TREE_OPTIMIZATION (opts)->x_optimize) + i = NULL; + if (i && i->local && i->can_change_signature) { int local_regparm, globals = 0, regno; --- gcc/testsuite/gcc.c-torture/execute/pr60062.c.jj 2014-02-05 09:02:13.703085235 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr60062.c 2014-02-05 09:01:54.000000000 +0100 @@ -0,0 +1,25 @@ +/* PR target/60062 */ + +int a; + +static void +foo (const char *p1, int p2) +{ + if (__builtin_strcmp (p1, "hello") != 0) + __builtin_abort (); +} + +static void +bar (const char *p1) +{ + if (__builtin_strcmp (p1, "hello") != 0) + __builtin_abort (); +} + +__attribute__((optimize (0))) int +main () +{ + foo ("hello", a); + bar ("hello"); + return 0; +} --- gcc/testsuite/gcc.c-torture/execute/pr60072.c.jj 2013-08-25 18:20:55.717911035 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr60072.c 2014-02-05 11:33:07.501748426 +0100 @@ -0,0 +1,16 @@ +/* PR target/60072 */ + +int c = 1; + +__attribute__ ((optimize (1))) +static int *foo (int *p) +{ + return p; +} + +int +main () +{ + *foo (&c) = 2; + return c - 2; +}