diff mbox

breakage with "[PATCH] combine: Disregard clobbers in another test for two SETs (PR65693)"

Message ID 20150409124117.GA24625@gate.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool April 9, 2015, 12:41 p.m. UTC 
On Wed, Apr 08, 2015 at 10:29:03PM -0400, Hans-Peter Nilsson wrote:
> On Wed, 8 Apr 2015, Segher Boessenkool wrote:
> > 2015-04-08  Segher Boessenkool  <segher@kernel.crashing.org>
> >
> > 	* combine.c (is_parallel_of_n_reg_sets): Change first argument
> > 	from an rtx_insn * to an rtx.
> > 	(try_combine): Adjust both callers.  Use it once more.

Hi,

> That "once more" is outside of #ifndef HAVE_cc0 and
> is_parallel_of_n_reg_sets is only defined inside of one.  Boom.

Oops.  So sorry.

> Full test on a cc0 target (such as cris-elf) is advised, and at
> least "make all-gcc" would be a minimum after fixing.

I tested a cross to cris-linux and built a Linux kernel with the trivial
patch (attached); doing that for all other cross configs is in progress.

It would be nice if there would be some cc0 target in the compile farm,
since cc0 isn't going away any time soon :-(

Okay if testing has finished successfully?


Segher


2015-04-09  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/65693
	* combine.c (is_parallel_of_n_reg_sets): Move outside of
	#ifndef HAVE_cc0.

---
 gcc/combine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jakub Jelinek April 9, 2015, 12:48 p.m. UTC | #1 
On Thu, Apr 09, 2015 at 07:41:17AM -0500, Segher Boessenkool wrote:
> It would be nice if there would be some cc0 target in the compile farm,
> since cc0 isn't going away any time soon :-(
> 
> Okay if testing has finished successfully?
> 
> 
> Segher
> 
> 
> 2015-04-09  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	PR rtl-optimization/65693
> 	* combine.c (is_parallel_of_n_reg_sets): Move outside of
> 	#ifndef HAVE_cc0.

This is obviously ok.

	Jakub
Steven Bosscher April 9, 2015, 1:21 p.m. UTC | #2 
On Thu, Apr 9, 2015 at 2:41 PM, Segher Boessenkool wrote:
> It would be nice if there would be some cc0 target in the compile farm,
> since cc0 isn't going away any time soon :-(

In this case it would be enough to replace the "#ifndef/#ifdef
HAVE_cc0" code with "if (HAVE_cc0)".

That's the simplest way to avoid compile breakage. Likewise for so
many other #ifdef code (HAVE_conditional_move, HAVE_trap, etc.).

Perhaps something to work on in the next stage1...

Ciao!
Steven
Jeff Law April 9, 2015, 2:15 p.m. UTC | #3 
On 04/09/2015 07:21 AM, Steven Bosscher wrote:
> On Thu, Apr 9, 2015 at 2:41 PM, Segher Boessenkool wrote:
>> It would be nice if there would be some cc0 target in the compile farm,
>> since cc0 isn't going away any time soon :-(
>
> In this case it would be enough to replace the "#ifndef/#ifdef
> HAVE_cc0" code with "if (HAVE_cc0)".
>
> That's the simplest way to avoid compile breakage. Likewise for so
> many other #ifdef code (HAVE_conditional_move, HAVE_trap, etc.).
>
> Perhaps something to work on in the next stage1...
Most definitely a direction I want to see us moving.  The glibc project 
recently went through the pain of the transition, but I believe it'll be 
worth it long term for them and even more so for us (we have a lot more 
conditionally compiled code than glibc).

jeff
Segher Boessenkool April 9, 2015, 2:29 p.m. UTC | #4 
On Thu, Apr 09, 2015 at 03:21:44PM +0200, Steven Bosscher wrote:
> On Thu, Apr 9, 2015 at 2:41 PM, Segher Boessenkool wrote:
> > It would be nice if there would be some cc0 target in the compile farm,
> > since cc0 isn't going away any time soon :-(
> 
> In this case it would be enough to replace the "#ifndef/#ifdef
> HAVE_cc0" code with "if (HAVE_cc0)".
> 
> That's the simplest way to avoid compile breakage. Likewise for so
> many other #ifdef code (HAVE_conditional_move, HAVE_trap, etc.).

If the code inside the #ifdef can actually compile for the opposite
condition, yeah.

The bad effect of not breaking compilation for cc0 targets is we are
even less likely to consider whether something would break on cc0 ;-)

> Perhaps something to work on in the next stage1...

Thanks for volunteering!


Segher
Hans-Peter Nilsson April 9, 2015, 2:44 p.m. UTC | #5 
On Thu, 9 Apr 2015, Segher Boessenkool wrote:
> I tested a cross to cris-linux and built a Linux kernel with the trivial
> patch (attached); doing that for all other cross configs is in progress.

Thanks.  Using contrib/config-list.mk comes to mind, but might
be a bit excessive in this particular case.

> It would be nice if there would be some cc0 target in the compile farm,
> since cc0 isn't going away any time soon :-(

Canned reply: use simulator targets,
<http://gcc.gnu.org/simtest-howto.html>.

brgds, H-P
diff mbox

Patch

diff --git a/gcc/combine.c b/gcc/combine.c
index 32950383..0836f74 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2492,7 +2492,6 @@  update_cfg_for_uncondjump (rtx_insn *insn)
     }
 }
 
-#ifndef HAVE_cc0
 /* Return whether PAT is a PARALLEL of exactly N register SETs followed
    by an arbitrary number of CLOBBERs.  */
 static bool
@@ -2517,6 +2516,7 @@  is_parallel_of_n_reg_sets (rtx pat, int n)
   return true;
 }
 
+#ifndef HAVE_cc0
 /* Return whether INSN, a PARALLEL of N register SETs (and maybe some
    CLOBBERs), can be split into individual SETs in that order, without
    changing semantics.  */