| Message ID | 6dd6eab124b41ccbae427674b36432a071fd7891.1436707794.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
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
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
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. */