On Tue, Oct 6, 2015 at 6:39 AM, Richard Biener <rguent...@suse.de> wrote:
> On Tue, 6 Oct 2015, H.J. Lu wrote:
>
>> On Tue, Oct 06, 2015 at 02:30:59PM +0200, Richard Biener wrote:
>> > On Tue, Oct 6, 2015 at 1:43 PM, H.J. Lu <hongjiu...@intel.com> wrote:
>> > > Since targetm.expand_to_rtl_hook may be called to switch ABI, it should
>> > > be called for each function before expanding to RTL. Otherwise, we may
>> > > use the stale information from compilation of the previous function.
>> > > aggregate_value_p uses call_used_regs. aggregate_value_p is used by
>> > > IPA and return value optimization, which are called before
>> > > pass_expand::execute after RTL expansion starts. We need to call
>> > > targetm.expand_to_rtl_hook early enough in cgraph_node::expand to make
>> > > sure that everything is in sync when RTL expansion starts.
>> > >
>> > > Tested on Linux/x86-64. OK for trunk?
>> >
>> > Hmm, I think set_cfun hook should handle this. expand_to_rtl_hook
>> > shouldn't
>> > mess with per-function stuff.
>> >
>> > Richard.
>> >
>>
>> I am testig this patch. OK for trunk if there is no regresion?
>>
>>
>> H.J.
>> --
>> ix86_maybe_switch_abi is called to late during RTL expansion and we
>> use the stale information from compilation of the previous function.
>> aggregate_value_p uses call_used_regs. aggregate_value_p is used by
>> IPA and return value optimization, which are called before
>> pass_expand::execute after RTL expansion starts. Instead,
>> ix86_maybe_switch_abi should be merged with ix86_set_current_function.
>>
>> PR target/67850
>> * config/i386/i386.c (ix86_set_current_function): Renamed
>> to ...
>> (ix86_set_current_function_1): This.
>> (ix86_set_current_function): New. incorporate old
>> ix86_set_current_function and ix86_maybe_switch_abi.
>> (ix86_maybe_switch_abi): Removed.
>> (TARGET_EXPAND_TO_RTL_HOOK): Likewise.
>> ---
>> gcc/config/i386/i386.c | 33 ++++++++++++++++++---------------
>> 1 file changed, 18 insertions(+), 15 deletions(-)
>>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index d59b59b..a0adf3d 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -6222,7 +6222,7 @@ ix86_reset_previous_fndecl (void)
>> FNDECL. The argument might be NULL to indicate processing at top
>> level, outside of any function scope. */
>> static void
>> -ix86_set_current_function (tree fndecl)
>> +ix86_set_current_function_1 (tree fndecl)
>> {
>> /* Only change the context if the function changes. This hook is called
>> several times in the course of compiling a function, and we don't want
>> to
>> @@ -6262,6 +6262,23 @@ ix86_set_current_function (tree fndecl)
>> ix86_previous_fndecl = fndecl;
>> }
>>
>> +static void
>> +ix86_set_current_function (tree fndecl)
>> +{
>> + ix86_set_current_function_1 (fndecl);
>> +
>> + if (!cfun)
>> + return;
>
> I think you want to test !fndecl here. Why split this out at all?
> The ix86_previous_fndecl caching should still work, no?
>
>> + /* 64-bit MS and SYSV ABI have different set of call used registers.
>> + Avoid expensive re-initialization of init_regs each time we switch
>> + function context since this is needed only during RTL expansion. */
>
> The comment is now wrong (and your bug shows it was wrong previously).
>
Here is the updated patch. OK for master if there is no
regression on Linux/x86-64?
--
H.J.
From e5c618c9f951d6b9e2c534d4ace9225c6c991c75 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Tue, 6 Oct 2015 05:50:37 -0700
Subject: [PATCH] Merge ix86_maybe_switch_abi with ix86_set_current_function
ix86_maybe_switch_abi is called to late during RTL expansion and we
use the stale information from compilation of the previous function.
aggregate_value_p uses call_used_regs. aggregate_value_p is used by
IPA and return value optimization, which are called before
ix86_maybe_switch_abi is called. This patch merges ix86_maybe_switch_abi
with ix86_set_current_function.
PR target/67850
* config/i386/i386.c (ix86_maybe_switch_abi): Merged with ...
(ix86_set_current_function): This.
(TARGET_EXPAND_TO_RTL_HOOK): Removed.
---
gcc/config/i386/i386.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 38953dd..c44f0af 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6367,6 +6367,14 @@ ix86_set_current_function (tree fndecl)
TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();
}
ix86_previous_fndecl = fndecl;
+
+ /* 64-bit MS and SYSV ABI have different set of call used registers.
+ Avoid expensive re-initialization of init_regs each time we switch
+ function context. */
+ if (TARGET_64BIT
+ && (call_used_regs[SI_REG]
+ == (cfun->machine->call_abi == MS_ABI)))
+ reinit_regs ();
}
@@ -7502,17 +7510,6 @@ ix86_call_abi_override (const_tree fndecl)
cfun->machine->call_abi = ix86_function_abi (fndecl);
}
-/* 64-bit MS and SYSV ABI have different set of call used registers. Avoid
- expensive re-initialization of init_regs each time we switch function context
- since this is needed only during RTL expansion. */
-static void
-ix86_maybe_switch_abi (void)
-{
- if (TARGET_64BIT &&
- call_used_regs[SI_REG] == (cfun->machine->call_abi == MS_ABI))
- reinit_regs ();
-}
-
/* Return 1 if pseudo register should be created and used to hold
GOT address for PIC code. */
bool
@@ -53911,9 +53908,6 @@ ix86_operands_ok_for_move_multiple (rtx *operands, bool load,
#undef TARGET_CAN_INLINE_P
#define TARGET_CAN_INLINE_P ix86_can_inline_p
-#undef TARGET_EXPAND_TO_RTL_HOOK
-#define TARGET_EXPAND_TO_RTL_HOOK ix86_maybe_switch_abi
-
#undef TARGET_LEGITIMATE_ADDRESS_P
#define TARGET_LEGITIMATE_ADDRESS_P ix86_legitimate_address_p
--
2.4.3
