public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 5/5] Ensure the mode used to create split registers is suppported
@ 2017-02-07 14:09 Matthew Fortune
  2017-02-07 22:04 ` Vladimir Makarov
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Fortune @ 2017-02-07 14:09 UTC (permalink / raw)
  To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
  Cc: Vladimir Makarov, Eric Botcazou (ebotcazou@adacore.com),
	Robert Suchanek, Moore, Catherine (Catherine_Moore@mentor.com)

Hi,

This patch addresses a problem with LRA splitting hard registers
where the mode requires multiple registers.  When splitting then
each constituent register is split individually using the widest
mode for each register but no check is made that such a mode is
actually supported in those registers.

MIPS has an ABI variant o32 FPXX that allows DFmode values to
exist in pairs of 32-bit floating point registers but only the
first 32-bit register is directly addressable.  The second
register can only be accessed as part of a 64-bit load/store or
via a special move instruction used as part of a 64-bit move.

The split is simply rejected to ensure compliance with the ABI
although I expect the split logic could account for this case
and split using the wider mode.  Such a change appears more
invasive than appropriate in stage 4.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78012

It is unknown if any other LRA enabled target could hit this
issue but it is certainly possible depending on mode/register
restrictions.

Thanks,
Matthew

Author: Robert Suchanek  <robert.suchanek@imgtec.com>

gcc/
	PR target/78012
	* lra-constraints.c (split_reg): Check requested split mode
	is supported by the register.
---
 gcc/lra-constraints.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 0b7ae34..db6e878 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -5402,6 +5402,26 @@ split_reg (bool before_p, int original_regno, rtx_insn *insn,
 	    }
 	  return false;
 	}
+      /* Split_if_necessary can split hard registers used as part of a
+	 multi-register mode but splits each register individually.  The
+	 mode used for each independent register may not be supported
+	 so reject the split.  Splitting the wider mode should theoretically
+	 be possible but is not implemented.  */
+      if (! HARD_REGNO_MODE_OK (hard_regno, mode))
+	{
+	  if (lra_dump_file != NULL)
+	    {
+	      fprintf (lra_dump_file,
+		       "    Rejecting split of %d(%s): unsuitable mode %s\n",
+		       original_regno,
+		       reg_class_names[lra_get_allocno_class (original_regno)],
+		       GET_MODE_NAME (mode));
+	      fprintf
+		(lra_dump_file,
+		 "    ))))))))))))))))))))))))))))))))))))))))))))))))\n");
+	    }
+	  return false;
+	}
       new_reg = lra_create_new_reg (mode, original_reg, rclass, "split");
       reg_renumber[REGNO (new_reg)] = hard_regno;
     }
-- 
2.2.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 5/5] Ensure the mode used to create split registers is suppported
  2017-02-07 14:09 [PATCH 5/5] Ensure the mode used to create split registers is suppported Matthew Fortune
@ 2017-02-07 22:04 ` Vladimir Makarov
  2017-02-20 12:41   ` Matthew Fortune
  0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Makarov @ 2017-02-07 22:04 UTC (permalink / raw)
  To: Matthew Fortune,
	'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
  Cc: Eric Botcazou (ebotcazou@adacore.com),
	Robert Suchanek, Moore, Catherine (Catherine_Moore@mentor.com)



On 02/07/2017 09:08 AM, Matthew Fortune wrote:
> Hi,
>
> This patch addresses a problem with LRA splitting hard registers
> where the mode requires multiple registers.  When splitting then
> each constituent register is split individually using the widest
> mode for each register but no check is made that such a mode is
> actually supported in those registers.
>
> MIPS has an ABI variant o32 FPXX that allows DFmode values to
> exist in pairs of 32-bit floating point registers but only the
> first 32-bit register is directly addressable.  The second
> register can only be accessed as part of a 64-bit load/store or
> via a special move instruction used as part of a 64-bit move.
>
> The split is simply rejected to ensure compliance with the ABI
> although I expect the split logic could account for this case
> and split using the wider mode.  Such a change appears more
> invasive than appropriate in stage 4.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78012
>
> It is unknown if any other LRA enabled target could hit this
> issue but it is certainly possible depending on mode/register
> restrictions.
The patch is ok for the trunk and it is pretty safe.  Thank you Robert 
and Matt.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH 5/5] Ensure the mode used to create split registers is suppported
  2017-02-07 22:04 ` Vladimir Makarov
@ 2017-02-20 12:41   ` Matthew Fortune
  0 siblings, 0 replies; 3+ messages in thread
From: Matthew Fortune @ 2017-02-20 12:41 UTC (permalink / raw)
  To: Vladimir Makarov,
	'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
  Cc: Eric Botcazou (ebotcazou@adacore.com),
	Robert Suchanek, Moore, Catherine (Catherine_Moore@mentor.com)

Vladimir Makarov <vmakarov@redhat.com> writes:
> On 02/07/2017 09:08 AM, Matthew Fortune wrote:
> > Hi,
> >
> > This patch addresses a problem with LRA splitting hard registers where
> > the mode requires multiple registers.  When splitting then each
> > constituent register is split individually using the widest mode for
> > each register but no check is made that such a mode is actually
> > supported in those registers.
> >
> > MIPS has an ABI variant o32 FPXX that allows DFmode values to exist in
> > pairs of 32-bit floating point registers but only the first 32-bit
> > register is directly addressable.  The second register can only be
> > accessed as part of a 64-bit load/store or via a special move
> > instruction used as part of a 64-bit move.
> >
> > The split is simply rejected to ensure compliance with the ABI
> > although I expect the split logic could account for this case and
> > split using the wider mode.  Such a change appears more invasive than
> > appropriate in stage 4.
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78012
> >
> > It is unknown if any other LRA enabled target could hit this issue but
> > it is certainly possible depending on mode/register restrictions.
> The patch is ok for the trunk and it is pretty safe.  Thank you Robert
> and Matt.

Committed as r245601.

Thanks,
Matthew

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-02-20 12:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-07 14:09 [PATCH 5/5] Ensure the mode used to create split registers is suppported Matthew Fortune
2017-02-07 22:04 ` Vladimir Makarov
2017-02-20 12:41   ` Matthew Fortune

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).