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]

[PATCH, rtl]: Fix PR rtl-optimization/35604


Hello!

There is a problem with compare-and-branch instructions when edge is
split and jump insn is updated by redirecting jump target in case jump
target label is also used inside compare operation.

As shown in the PR [1], compiling dnrm2.f we generate this asm:

	...
	movl	%eax, -28(%ebp)
	jle	.L30
*	movl	$.L3, %edx
	movl	$1, %ebx
	sall	$3, %eax
*	cmpl	$.L3, %edx
	movl	$1, -24(%ebp)
	fldz
	movl	%eax, -20(%ebp)
**	je	.L31
	...
.L31:
	fstp	%st(1)
.L3:
	movl	-24(%ebp), %ecx
	...

In this example, jump insn (**) was updated from .L3 to .L31 due to
edge split and following jump target redirection. Please note, how
insns marked with (*) load and compare the same value.

Adding my experimental patch [2] to fuse compare and branch
instruction in order to use core2's macro-ops fusing feature, the code
changes to:

	...
	movl	%eax, -28(%ebp)
	jle	.L30
*	movl	$.L3, %edx
	movl	$1, -24(%ebp)
	movl	$1, %ebx
	fldz
	movl	%eax, -20(%ebp)
	.p2align 4,,7
!*	cmpl	$.L31, %edx
!**	je	.L31	# fused
	...
.L31:
	fstp	%st(1)
.L3:
	movl	-24(%ebp), %ecx
	...

Please note, that insn, marked with (*) doesn't load and compare the
same value, leading to runtime abort. The problem is in the way that
current RTL handling code handles jump target redirection. The code
blindly changes _all_ old jump target labels in jump_insn RTX to the
new jump target labels, including the label that is used in compare to
compare against. As shown in the example above, this is wrong thing to
do, an only strict jump target should be changed.

Attached patch fixes this problem by skipping first operand of
IF_THEN_ELSE insn entirely. Patched gcc then produces expected code
that corresponds to the original code:

	...
	movl	%eax, -28(%ebp)
	jle	.L30
*	movl	$.L3, %edx
	movl	$1, -24(%ebp)
	movl	$1, %ebx
	fldz
	movl	%eax, -20(%ebp)
	.p2align 4,,7
*	cmpl	$.L3, %edx
**	je	.L31	# fused
	...
.L31:
	fstp	%st(1)
.L3:
	movl	-24(%ebp), %ecx
	...

2008-06-18  Uros Bizjak  <ubizjak@gmail.com>

	PR rtl-optimization/35604
	* jump.c (redirect_exp_1): Skip processing of the first operand of
	IF_THEN_ELSE RTX to avoid changing compare operands in case old code
	label is an operand to compare against.

The patch was bootstrapped and regression tested on i686-pc-linux-gnu
together with fused-ops patch as in [2]. This patch creates lots of
fused compare-and-branch instructions so it exercises the change a
lot. The test without fused ops patch is running right now /but
compare-and-branch insns are not produced without the patch on i686/.

OK for mainline if the test passes? What about 4.3?

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35604
[2] http://gcc.gnu.org/ml/gcc-patches/2008-03/msg00969.html

Uros.

Attachment: p.diff.txt
Description: Text document


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