* What is this patch doing?
@ 2000-08-01 12:09 Bernd Schmidt
2000-08-01 12:35 ` Joern Rennecke
0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2000-08-01 12:09 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc
I found that the patch below seems to be at least partially responsible for
the massive testsuite failures I'm seeing for the sh. I couldn't find a
discussion of it in any of the mailing list archives (searching for "Must be
split", "output_branchy_insn" and "cmpgt"). Why was it installed?
The problem appears to be that there's no JUMP_LABEL for the newly
generated jump. Is it valid for splitters to create new basic blocks?
The failure can be observed by compiling c-torture/execute/950612-1.c with
"-m3". We get a segfault from cc1. This goes away if I revert this patch.
Bernd
Tue Jul 25 23:08:33 2000 J"orn Rennecke <amylaar@cygnus.co.uk>
* sh.md (cmpgtdi_t): Must be split.
(cmpgtdi_t+1): New splitter.
--- /home/bernds/work/devo/gcc/config/sh/sh.md Mon Jul 31 22:52:07 2000
+++ config/sh/sh.md Tue Aug 1 20:22:23 2000
@@ -627,15 +627,35 @@
[(set (reg:SI 18) (eq:SI (match_operand:DI 0 "arith_reg_operand" "r,r")
(match_operand:DI 1 "arith_reg_or_0_operand" "N,r")))]
""
- "*
- return output_branchy_insn
- (EQ,
- (which_alternative
- ? \"cmp/eq\\t%S1,%S0\;bf\\t%l9\;cmp/eq\\t%R1,%R0\"
- : \"tst\\t%S0,%S0\;bf\\t%l9\;tst\\t%R0,%R0\"),
- insn, operands);"
+ "#"
[(set_attr "length" "6")
(set_attr "type" "arith3b")])
+
+(define_split
+ [(set (reg:SI 18) (eq:SI (match_operand:DI 0 "arith_reg_operand" "r,r")
+ (match_operand:DI 1 "arith_reg_or_0_operand" "N,r")))]
+ "reload_completed"
+ [(set (reg:SI 18) (eq:SI (match_dup 2) (match_dup 3)))
+ (set (pc) (if_then_else (ne (reg:SI 18) (const_int 0))
+ (label_ref (match_dup 6))
+ (pc)))
+ (set (reg:SI 18) (eq:SI (match_dup 4) (match_dup 5)))
+ (match_dup 6)]
+ "
+{
+ operands[2]
+ = gen_rtx_REG (SImode,
+ true_regnum (operands[0]) + (TARGET_LITTLE_ENDIAN ? 1 : 0));
+ operands[3]
+ = (operands[1] == const0_rtx
+ ? const0_rtx
+ : gen_rtx_REG (SImode,
+ true_regnum (operands[1])
+ + (TARGET_LITTLE_ENDIAN ? 1 : 0)));
+ operands[4] = gen_lowpart (SImode, operands[0]);
+ operands[5] = gen_lowpart (SImode, operands[1]);
+ operands[6] = gen_label_rtx ();
+}")
(define_insn "cmpgtdi_t"
[(set (reg:SI 18) (gt:SI (match_operand:DI 0 "arith_reg_operand" "r,r")
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: What is this patch doing?
2000-08-01 12:09 What is this patch doing? Bernd Schmidt
@ 2000-08-01 12:35 ` Joern Rennecke
2000-08-01 12:36 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2000-08-01 12:35 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: gcc, rth
> I found that the patch below seems to be at least partially responsible for
> the massive testsuite failures I'm seeing for the sh. I couldn't find a
> discussion of it in any of the mailing list archives (searching for "Must be
> split", "output_branchy_insn" and "cmpgt"). Why was it installed?
Well, the SH port didn't even build before. It was failing to build some
part of libgcc.a
> The problem appears to be that there's no JUMP_LABEL for the newly
> generated jump. Is it valid for splitters to create new basic blocks?
We talked in the thread '(i386-linux x sh-elf) build breakage' on gcc-bugs
about it. Richard Henderson said that the first post-reload splitter
could generate new blocks.
> The failure can be observed by compiling c-torture/execute/950612-1.c with
> "-m3". We get a segfault from cc1. This goes away if I revert this patch.
Do I need to pass any other flags to cc1? Or is this dependent on some
patch that was checked in after mine?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: What is this patch doing?
2000-08-01 12:35 ` Joern Rennecke
@ 2000-08-01 12:36 ` Bernd Schmidt
2000-08-01 14:04 ` Joern Rennecke
0 siblings, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2000-08-01 12:36 UTC (permalink / raw)
To: Joern Rennecke; +Cc: gcc, rth
On Tue, 1 Aug 2000, Joern Rennecke wrote:
>
> > The failure can be observed by compiling c-torture/execute/950612-1.c with
> > "-m3". We get a segfault from cc1. This goes away if I revert this patch.
>
> Do I need to pass any other flags to cc1? Or is this dependent on some
> patch that was checked in after mine?
Just -m3 is enough to reproduce it here.
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: What is this patch doing?
2000-08-01 12:36 ` Bernd Schmidt
@ 2000-08-01 14:04 ` Joern Rennecke
2000-08-01 14:42 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Joern Rennecke @ 2000-08-01 14:04 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: Joern Rennecke, gcc, rth
> Just -m3 is enough to reproduce it here.
I have updated my tree now and can reporduce the problem.
There are actually two problem with using the first post-reload
splitting pass: first, rebuild_notes_after_reload is not set
depending on whether the splitter created any notes or not.
But, more importantly, this splitter is only run when optimizing.
So a split pattern that is only supposed to be used at that point
would have to be guarded against being run too late. Do we want to
introduce a new global variable that says when a split pattern may
generate labels? Or is there already one?
Or should we rather make this post-reload splitter always executed?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: What is this patch doing?
2000-08-01 14:04 ` Joern Rennecke
@ 2000-08-01 14:42 ` Richard Henderson
2000-08-01 15:03 ` Joern Rennecke
0 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2000-08-01 14:42 UTC (permalink / raw)
To: Joern Rennecke; +Cc: Bernd Schmidt, Joern Rennecke, gcc
On Tue, Aug 01, 2000 at 10:04:14PM +0100, Joern Rennecke wrote:
> But, more importantly, this splitter is only run when optimizing.
> So a split pattern that is only supposed to be used at that point
> would have to be guarded against being run too late. Do we want to
> introduce a new global variable that says when a split pattern may
> generate labels? Or is there already one?
There isn't one at present.
But iirc this splitter, and the branchy widgetry it replaced was an
optimization was it not? There was a simple jump around sequence
when it couldn't figure out what to do. Can you just output that
(without splitting) at -O0?
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: What is this patch doing?
2000-08-01 14:42 ` Richard Henderson
@ 2000-08-01 15:03 ` Joern Rennecke
0 siblings, 0 replies; 6+ messages in thread
From: Joern Rennecke @ 2000-08-01 15:03 UTC (permalink / raw)
To: Richard Henderson; +Cc: Joern Rennecke, Bernd Schmidt, gcc
> But iirc this splitter, and the branchy widgetry it replaced was an
> optimization was it not? There was a simple jump around sequence
> when it couldn't figure out what to do. Can you just output that
> (without splitting) at -O0?
Well, I could, but there are actually six patterns affected overall.
this means that the output pattern would be repeated in a non-splitted
form for each of these patterns.
And the splitters would all have to have a predicate
"reload_completed && optimize_size" .
Maybe we should rather fix the problem by changing split_all_insns so that
it calls rebuild_jump_labels when needed.
Or change try_split to call rebuild_jump_labels on the result - that
would even cover the case where final splits an insn just before outputting.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2000-08-01 15:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2000-08-01 12:09 What is this patch doing? Bernd Schmidt
2000-08-01 12:35 ` Joern Rennecke
2000-08-01 12:36 ` Bernd Schmidt
2000-08-01 14:04 ` Joern Rennecke
2000-08-01 14:42 ` Richard Henderson
2000-08-01 15:03 ` Joern Rennecke
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).