diff mbox

[i386] : RFC enable inlining for function with machine-attributes

Message ID CAEwic4Z-3AF+mmkC0MVMGRnQGYpATD=zLwHKO+i2GCo_u5umfA@mail.gmail.com
State New
Headers show

Commit Message

Kai Tietz June 17, 2013, 6:55 a.m. UTC
Hi,

I am working right now on PR/56892, which is about missing
inline-optimization for dllexported classes.  That's caused by the
default for TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P hook, which
disallows inlining for any machine-attribute.  By taking a closer look
to i386's attributes, I don't see a good reason why we disallow them
for machine-attributes listed in TARGET_ATTRIBUTE_TABLE.

In general this issue happens also for inline-functions in classes
which are prominent marked with a specific ABI, or calling-convention.
 So I think this issue isn't windows targets specific.  By
performance-tests it shows that by this missed optimization for
mingw/cygwin targets on dllexported-classes is causing about 4-times
speed-penalty in comparison to other target c++-compilers.

So I suggest the following patch:

ChangeLog

2013-06-17  Kai Tietz  <ktietz@redhat.com>

       PR target/56892
        * config/i386/i386.c (ix86_function_attribute_inlinable_p): New local
        function.
        (TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P): Define as
        ix86_function_attribute_inlinable_p.

Tested for x86_64-w64-mingw32, i686-w64-mingw32, and x86_64-unknown-linux-gnu.

Ok for apply?

 #  define TARGET_MERGE_DECL_ATTRIBUTES merge_dllimport_decl_attributes

Comments

Richard Henderson June 17, 2013, 4:19 p.m. UTC | #1
On 06/16/2013 11:55 PM, Kai Tietz wrote:
> +static bool
> +ix86_function_attribute_inlinable_p (const_tree fndecl ATTRIBUTE_UNUSED)
> +{
> +  return true;
> +}

This is hook_bool_const_tree_true.

I have an idea that perhaps the default ought to be true, and that the few
targets (like mep) that have an interrupt attribute, etc ought to be the ones
to actually implement this hook.


r~
Kai Tietz June 17, 2013, 5:36 p.m. UTC | #2
2013/6/17 Richard Henderson <rth@redhat.com>:
> On 06/16/2013 11:55 PM, Kai Tietz wrote:
>> +static bool
>> +ix86_function_attribute_inlinable_p (const_tree fndecl ATTRIBUTE_UNUSED)
>> +{
>> +  return true;
>> +}
>
> This is hook_bool_const_tree_true.

Right, we could define macro to this ...

> I have an idea that perhaps the default ought to be true, and that the few
> targets (like mep) that have an interrupt attribute, etc ought to be the ones
> to actually implement this hook.

Yes, that was actual the cause why I did a RFC on that patch.  I was
concerned to break some targets by changing the default.  I admit that
changing default would be the preferred solution for me, too.

>
> r~

Kai
diff mbox

Patch

Index: i386.c
===================================================================
--- i386.c      (Revision 200128)
+++ i386.c      (Arbeitskopie)
@@ -38721,6 +38721,12 @@  static const struct attribute_spec ix86_attribute_
   { NULL,        0, 0, false, false, false, NULL, false }
 };

+static bool
+ix86_function_attribute_inlinable_p (const_tree fndecl ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
 /* Implement targetm.vectorize.builtin_vectorization_cost.  */
 static int
 ix86_builtin_vectorization_cost (enum vect_cost_for_stmt type_of_cost,
@@ -42700,6 +42706,8 @@  ix86_memmodel_check (unsigned HOST_WIDE_INT val)

 #undef TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE ix86_attribute_table
+#undef TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
+#define TARGET_FUNCTION_ATTRIBUTE_INLINABLE_P
ix86_function_attribute_inlinable_p
 #if TARGET_DLLIMPORT_DECL_ATTRIBUTES
 #  undef TARGET_MERGE_DECL_ATTRIBUTES