Patchwork [i386] Fix PR 57756

login
register
mail settings
Submitter Sriraman Tallam
Date Aug. 14, 2013, 12:02 a.m.
Message ID <CAAs8HmykvBN9u=69ZxEHpC5C1NDr6xAWsCrp2tRCiX+NgWf7GA@mail.gmail.com>
Download mbox | patch
Permalink /patch/266941/
State New
Headers show

Comments

Sriraman Tallam - Aug. 14, 2013, 12:02 a.m.
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.
Sriraman Tallam - Aug. 14, 2013, 5:39 p.m.
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
>
>

Patch

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