This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Correct fix for scheduler bug PR11320


PR11320 was a scheduler problem where an instruction was moved backwards
across a branch, leading to
	addl r14 = @ltoffx(.LC2), r1
	;;
	(p7) addl r14 = 1, r0		<--- r14 clobbered
	;;
	(p7) st4 [r15] = r14
	ld8.mov r14 = [r14], .LC2	<--- crash
	(branch was here)

At the time, this was solved by a patch that recognizes when a register
is conditionally set in a basic block, and then treats the branch as
setting such a register:

http://gcc.gnu.org/ml/gcc-patches/2003-07/msg01044.html

"The proposed fix is to say that JUMP_INSNs set the registers that are
live on entry of the fallthrough block and are conditionally set before
the jump, because they can be considered as restoring the former value
of the conditionally set registers."

While it is true that a jump could be seen as setting the correct value
for a register, this isn't at all related to conditional sets. Consider
a trivial example:

a = b + 3;
if (b > 0) { use a } else { use a }

where the value of a is different in each branch of the else, with no
conditional execution in sight. From the point of view of the uses, it
can be argued that the jump sets the value, but this is irrelevant for
scheduling purposes.

There's nothing wrong with using a wrong value in an instruction hoisted
across a branch if the result of that instruction will (eventually) be
dead. We do this all the time in sched-ebb. The patch needlessly
restricts scheduling opportunities and should be reverted.

The real problem here is that the ia64 backend lies to the rest of the
compiler; it produces a load instruction without showing a MEM anywhere
in the instruction pattern. Hence, the following patch, which reverts
the bogus scheduler changes and adds a MEM to a pattern in ia64.md. The
represenation is somewhat weird, but no more so than before where the
load was represented as a plain LO_SUM. The documentation I have doesn't
mention ld8.mov, so I'll have to leave it to the ia64 maintainers to
choose between this change or implement something more involved.

Bootstrapped and tested without regressions on ia64-linux with
languages=c,c++,obj,fortran and --disable-shared; that was what I could
get to work at all at the moment. Even then, C++ seems pretty broken.
I've also built a 3.3 cross-cc1 to see if the original bug is still
fixed (it is).

Ok (scheduler & ia64 bits)?


Bernd

Attachment: pr11320.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]