public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Uros Bizjak <ubizjak@gmail.com>,	Jeff Law <law@redhat.com>,
	Richard Biener <rguenther@suse.de>
Subject: [PATCH] PR target/67850: Wrong call_used_regs used in aggregate_value_p
Date: Tue, 06 Oct 2015 13:30:00 -0000	[thread overview]
Message-ID: <20151006133051.GA25074@gmail.com> (raw)
In-Reply-To: <CAFiYyc14wu3wziksJr3Gm38jD0xqad3d9raKnQh8cSbAXGyj1Q@mail.gmail.com>

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.lu@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;
+
+  /* 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.  */
+  if (TARGET_64BIT
+      && (call_used_regs[SI_REG]
+	  == (cfun->machine->call_abi == MS_ABI)))
+    reinit_regs ();
+}
+
 \f
 /* Return true if this goes in large data/bss.  */
 
@@ -7395,17 +7412,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
@@ -53802,9 +53808,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

  reply	other threads:[~2015-10-06 13:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-06 11:43 [PATCH] PR middle-end/67850: " H.J. Lu
2015-10-06 12:31 ` Richard Biener
2015-10-06 13:30   ` H.J. Lu [this message]
2015-10-06 13:39     ` [PATCH] PR target/67850: " Richard Biener
2015-10-06 15:01       ` H.J. Lu
2015-10-06 17:09         ` H.J. Lu
2015-10-07  8:53           ` Richard Biener
2015-10-07  9:01             ` Uros Bizjak
2015-10-12 12:50               ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151006133051.GA25074@gmail.com \
    --to=hjl.tools@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    --cc=rguenther@suse.de \
    --cc=richard.guenther@gmail.com \
    --cc=ubizjak@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).