| Message ID | CAAs8HmykvBN9u=69ZxEHpC5C1NDr6xAWsCrp2tRCiX+NgWf7GA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 14, 2013 at 3:38 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Wed, Aug 14, 2013 at 2:02 AM, Sriraman Tallam <tmsriram@google.com> > wrote: >> >> Hi, >> >> I have attached a patch to fix PR57756. Description: The >> following program, >> >> __attribute__((always_inline,target("sse4.2"))) >> __inline int callee () >> { >> return 0; >> } >> >> __attribute__((target("sse"))) >> __inline int caller () >> { >> return callee(); >> } >> >> does not generate an error and callee is inlined into caller. This is >> because callee has a higher target ISA. Interchanging the position of >> caller and callee will generate the correct error. Also, removing the >> target attribute from caller will generate the error. >> >> The reason for the bug is that when the caller's target options are >> processed, global_options contain the ISA flags of the callee >> (previously processed) and doing this in i386-common.c, where opts is >> global_options: >> >> opts->x_ix86_isa_flags |= OPTION_MASK_ISA_SSE_SET; >> >> is not changing global options. The fix is to reset global_options to >> the default each time a new target option needs to be processed. > > > Shouldn't ix86_valid_target_attribute_tree be refactored to work on a > selected > opt structure rather than global_options? Yes, that would be the ideal fix. The code here seems hairy and many functions set and use global_options directly (in the call chain of ix86_valid_target_attribute_tree). I will take a stab at it. Thanks Sri > > Richard. > >> >> Patch ok? >> >> Thanks >> Sri > >
Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 201701) +++ config/i386/i386.c (working copy) @@ -4879,6 +4879,10 @@ ix86_valid_target_attribute_p (tree fndecl, /* The target attributes may also change some optimization flags, so update the optimization options if necessary. */ cl_target_option_save (&cur_target, &global_options); + /* ix86_valid_target_attribute_tree sets global_options. Restore + global_options to the default before its target options can be set. */ + cl_target_option_restore (&global_options, + TREE_TARGET_OPTION (target_option_default_node)); new_target = ix86_valid_target_attribute_tree (args); new_optimize = build_optimization_node (); Index: testsuite/gcc.target/i386/pr57756.c =================================================================== --- testsuite/gcc.target/i386/pr57756.c (revision 0) +++ testsuite/gcc.target/i386/pr57756.c (revision 0) @@ -0,0 +1,20 @@ +/* callee cannot be inlined into caller because it has a higher + target ISA. */ +/* { dg-do compile } */ + +__attribute__((always_inline,target("sse4.2"))) +__inline int callee () /* { dg-error "inlining failed in call to always_inline" } */ +{ + return 0; +} + +__attribute__((target("sse"))) +__inline int caller () +{ + return callee(); /* { dg-error "called from here" } */ +} + +int main () +{ + return caller(); +}
Hi, I have attached a patch to fix PR57756. Description: The following program, __attribute__((always_inline,target("sse4.2"))) __inline int callee () { return 0; } __attribute__((target("sse"))) __inline int caller () { return callee(); } does not generate an error and callee is inlined into caller. This is because callee has a higher target ISA. Interchanging the position of caller and callee will generate the correct error. Also, removing the target attribute from caller will generate the error. The reason for the bug is that when the caller's target options are processed, global_options contain the ISA flags of the callee (previously processed) and doing this in i386-common.c, where opts is global_options: opts->x_ix86_isa_flags |= OPTION_MASK_ISA_SSE_SET; is not changing global options. The fix is to reset global_options to the default each time a new target option needs to be processed. Patch ok? Thanks Sri * config/i386/i386.c (ix86_valid_target_attribute_p): Restore global_options to default before modifying it. * gcc.target/i386/pr57756.c: New test.