public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix expansion ICE on stores into parts of hard registers (PR middle-end/80173)
@ 2017-03-30 21:33 Jakub Jelinek
  2017-03-31  6:27 ` Jeff Law
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2017-03-30 21:33 UTC (permalink / raw)
  To: Jeff Law, Bernd Schmidt; +Cc: gcc-patches

Hi!

On some arches like x86_64 we allow some aggregates in named register
variables.  If those registers are wider than word, store_bit_field_1
was assuming it must be multiple registers and attempted to pick
a word sized subregister of it, which is fine for pseudos, but if the
destination is a hard register, sometimes such subreg will fail.
If it is a hard register and we know that its mode means a single
register (otherwise we wouldn't allow creating a named register variable for
it), then that subreg is certainly counter-productive, we just want to store
into the whole register.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-03-30  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/80173
	* expmed.c (store_bit_field_1): Don't attempt to create
	a word subreg out of hard registers wider than word if they
	have HARD_REGNO_NREGS of 1 for their mode.

	* gcc.target/i386/pr80173.c: New test.

--- gcc/expmed.c.jj	2017-01-01 12:45:39.000000000 +0100
+++ gcc/expmed.c	2017-03-30 14:51:01.814449691 +0200
@@ -965,8 +965,14 @@ store_bit_field_1 (rtx str_rtx, unsigned
     }
 
   /* If OP0 is a multi-word register, narrow it to the affected word.
-     If the region spans two words, defer to store_split_bit_field.  */
-  if (!MEM_P (op0) && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD)
+     If the region spans two words, defer to store_split_bit_field.
+     Don't do this if op0 is a single hard register wider than word
+     such as a float or vector register.  */
+  if (!MEM_P (op0)
+      && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD
+      && (!REG_P (op0)
+	  || !HARD_REGISTER_P (op0)
+	  || HARD_REGNO_NREGS (REGNO (op0), GET_MODE (op0)) != 1))
     {
       if (bitnum % BITS_PER_WORD + bitsize > BITS_PER_WORD)
 	{
--- gcc/testsuite/gcc.target/i386/pr80173.c.jj	2017-03-30 15:05:49.600889474 +0200
+++ gcc/testsuite/gcc.target/i386/pr80173.c	2017-03-30 15:05:33.000000000 +0200
@@ -0,0 +1,22 @@
+/* PR middle-end/80173 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -ffixed-xmm7" } */
+
+typedef int V __attribute__ ((vector_size (2 * sizeof (int))));
+
+struct U { V a; V b; };
+
+int
+foo (int i)
+{
+  register struct U u asm ("xmm7") = {{-1, 0}, {-1, 0}};
+  return u.b[i];
+}
+
+int
+bar (int i)
+{
+  register struct U u asm ("xmm7");
+  u = (struct U) {{-1, 0}, {-1, 0}};
+  return u.b[i];
+}

	Jakub

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

* Re: [PATCH] Fix expansion ICE on stores into parts of hard registers (PR middle-end/80173)
  2017-03-30 21:33 [PATCH] Fix expansion ICE on stores into parts of hard registers (PR middle-end/80173) Jakub Jelinek
@ 2017-03-31  6:27 ` Jeff Law
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff Law @ 2017-03-31  6:27 UTC (permalink / raw)
  To: Jakub Jelinek, Bernd Schmidt; +Cc: gcc-patches

On 03/30/2017 02:38 PM, Jakub Jelinek wrote:
> Hi!
>
> On some arches like x86_64 we allow some aggregates in named register
> variables.  If those registers are wider than word, store_bit_field_1
> was assuming it must be multiple registers and attempted to pick
> a word sized subregister of it, which is fine for pseudos, but if the
> destination is a hard register, sometimes such subreg will fail.
> If it is a hard register and we know that its mode means a single
> register (otherwise we wouldn't allow creating a named register variable for
> it), then that subreg is certainly counter-productive, we just want to store
> into the whole register.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-03-30  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR middle-end/80173
> 	* expmed.c (store_bit_field_1): Don't attempt to create
> 	a word subreg out of hard registers wider than word if they
> 	have HARD_REGNO_NREGS of 1 for their mode.
>
> 	* gcc.target/i386/pr80173.c: New test.
I could easily argue that we shouldn't ever create subregs of hardregs. 
They should either be rejected or simplified into normal reg 
expressions.  There's been many bugs in the past due to these kinds of 
problems.

This patch clearly fits that general guidance.

OK for the trunk and any release branches where you want to backport it.

jeff

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

end of thread, other threads:[~2017-03-31  6:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-30 21:33 [PATCH] Fix expansion ICE on stores into parts of hard registers (PR middle-end/80173) Jakub Jelinek
2017-03-31  6:27 ` Jeff Law

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