[PATCH, RFC] combine: Don't create insv insns unless HAVE_insv

[PATCH, RFC] combine: Don't create insv insns unless HAVE_insv

[Segher Boessenkool]

Currently combine tries to make assignments to bitfields (of a register)
whenever it can.  If the target has no insv pattern, the result will not
ever match (if the MD is sane at all).  Doing insv on registers generates
worse code than what you get if you express things directly (with and/ior),
so many targets do not _want_ to have insv patterns.

This patch changes combine to not generate insv patterns if the target
does not have any.

Bootstrapped and regression checked on powerpc64-linux (with and without
insv patterns there).  Also built on many other targets, for many months.

I'm vaguely aware there have been changes to extzv etc. so there now are
extzv<mode>; I'll investigate if that means anything for insv as well.
It's also a new #ifdef HAVE_xxx.  But we're not clean there yet so I hope
to get away with that ;-)

Comments?  Complaints?


Segher

---
 gcc/combine.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcc/combine.c b/gcc/combine.c
index 9be230a..dc51d51 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -7488,6 +7488,13 @@ make_extraction (machine_mode mode, rtx inner, HOST_WIDE_INT pos,
 			     mode, new_rtx));
     }
 
+  /* If the target has no INSV patterns, do not try to generate such an
+     instruction.  */
+#ifndef HAVE_insv
+  if (in_dest)
+    return 0;
+#endif
+
   /* Unless this is a COMPARE or we have a funny memory reference,
      don't do anything with zero-extending field extracts starting at
      the low-order bit since they are simple AND operations.  */
-- 
1.8.1.4

Re: [PATCH, RFC] combine: Don't create insv insns unless HAVE_insv

[Jeff Law]

On 07/12/2015 07:56 AM, Segher Boessenkool wrote:
> Currently combine tries to make assignments to bitfields (of a register)
> whenever it can.  If the target has no insv pattern, the result will not
> ever match (if the MD is sane at all).  Doing insv on registers generates
> worse code than what you get if you express things directly (with and/ior),
> so many targets do not _want_ to have insv patterns.
>
> This patch changes combine to not generate insv patterns if the target
> does not have any.
>
> Bootstrapped and regression checked on powerpc64-linux (with and without
> insv patterns there).  Also built on many other targets, for many months.
>
> I'm vaguely aware there have been changes to extzv etc. so there now are
> extzv<mode>; I'll investigate if that means anything for insv as well.
> It's also a new #ifdef HAVE_xxx.  But we're not clean there yet so I hope
> to get away with that ;-)
>
> Comments?  Complaints?
Well, I'd rather avoid the #ifdef.  Just because we aren't clean yet 
doesn't mean we need to introduce more stuff to clean up later.

It'd also be nice to have a testcase or two.  Guessing they'd be target 
dependent, but that's OK with me.

jeff

Re: [PATCH, RFC] combine: Don't create insv insns unless HAVE_insv

[Segher Boessenkool]

On Mon, Jul 13, 2015 at 10:50:28PM -0600, Jeff Law wrote:
> On 07/12/2015 07:56 AM, Segher Boessenkool wrote:
> >Currently combine tries to make assignments to bitfields (of a register)
> >whenever it can.  If the target has no insv pattern, the result will not
> >ever match (if the MD is sane at all).  Doing insv on registers generates
> >worse code than what you get if you express things directly (with and/ior),
> >so many targets do not _want_ to have insv patterns.
> >
> >This patch changes combine to not generate insv patterns if the target
> >does not have any.
> >
> >Bootstrapped and regression checked on powerpc64-linux (with and without
> >insv patterns there).  Also built on many other targets, for many months.
> >
> >I'm vaguely aware there have been changes to extzv etc. so there now are
> >extzv<mode>; I'll investigate if that means anything for insv as well.
> >It's also a new #ifdef HAVE_xxx.  But we're not clean there yet so I hope
> >to get away with that ;-)
> >
> >Comments?  Complaints?

> Well, I'd rather avoid the #ifdef.  Just because we aren't clean yet 
> doesn't mean we need to introduce more stuff to clean up later.

This patch is very old, from long before the HAVE_* conversion ;-)
I'll clean it up, the insv<mode> needs handling anyway.

> It'd also be nice to have a testcase or two.  Guessing they'd be target 
> dependent, but that's OK with me.

I can make some for the powerpc insert things, when that goes in.  What you
need to test is that combine does *not* create insv from thin air, so that
it _can_ create other things.  It is pretty hard to test if things are *not*
done :-)


Segher