Subject: Re: tools build on ELF mac68k
To: Steve Woodford <steve@mctavish.co.uk>
From: Matthew Fredette <fredette@MIT.EDU>
List: tech-toolchain
Date: 12/06/2001 11:45:20
> As for the New Toolchain on m68k, there are two problems preventing it
> going live:
> 
> 1) The compiler mis-optimises with -fstrength-reduce enabled (which is the
> default for -O2). I've asked Mycroft to look into this, but anyone with
> cc1 clue is welcome to track down the cause. Hint: It also seems to affect
> Vax and, on both platforms, causes cc1 to SEGV when compiling
> sys/dev/scsipi/ch.c ...

I decided to take a shot at this.  After staring at the strength
reduction code in loop.c for a while, I have a guess about what
the problem is.  

Things blow up when strength_reduce calls reg_used_between_p on two
insns A and B, where A comes before B in "loop insn order" (i.e., as
next_insn_in_loop saw things), but A actually comes *after* B in
NEXT_INSN order.  Since reg_used_between_p just walks the insn chain
in NEXT_INSN order starting at A and looking for B, it never finds it
and walks off the end of the chain.

This situation happens when the strength reduction is told that 
the loop is to be entered near the end of the loop (strength_reduce
is called with a non-NULL loop_top).  In this case the biv-searching
code can come to choose two instructions A and B as above.

I think what needs to happen is that strength_reduce needs to call
a version of reg_used_between_p that knows about "loop insn order".

Anyways, with that guess in mind I took a look at the problem `for'
loop at the end of the ch_ousergetelemstatus function in
sys/dev/scsipi/ch.c.  I decided that the loop was being entered at the
update of the `desc' local, and shuffled things around to try to
discourage that:

[snip]
Index: ch.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/scsipi/ch.c,v
retrieving revision 1.48
diff -u -r1.48 ch.c
--- ch.c	2001/11/13 06:56:39	1.48
+++ ch.c	2001/12/06 16:20:52
@@ -759,10 +759,10 @@
 	for (i = 0; i < avail; ++i) {
 		user_data = desc->flags1;
 		error = copyout(&user_data, &uptr[i], avail);
-		if (error)
-			break;
 		desc = (struct read_element_status_descriptor *)((u_long)desc
 		    + desclen);
+		if (error)
+			break;
 	}
 
  done:
[snip]

With this change, ch.c now compiles, although this should only be
seen as a stopgap measure.

I'm going to go ahead now and see if my guess is right and try to fix
the problem in gcc, but I wanted to see if this sounds remotely right
to anyone else looking at the problem.

Matt

--
Matt Fredette
http://mit.edu/fredette/www