minor code-quality regression vs. 2.95

[Clinton Popetz]

On Tue, Apr 11, 2000 at 09:45:41PM -0700, Zack Weinberg wrote:
> This function:
> 
> long long blocks (long long bytes) { return bytes / 512; }
> 
> compiles as follows with 2.95:
> 
> blocks:
> 	movl 4(%esp),%eax
> 	movl 8(%esp),%edx
> 	testl %edx,%edx
> 	jge .L3
> 	addl $511,%eax
> 	adcl $0,%edx
> L3:
> 	shrdl $9,%edx,%eax
> 	sarl $9,%edx
> 	ret
> 
> and with the 20000408 CVS tree:
> 
> blocks:
> 	movl	8(%esp), %edx
> 	movl	4(%esp), %eax
> 	cmpl	$0, %edx
> 	jg	.L3
> 	testl	%edx, %edx
> 	jns	.L3
> 	addl	$511, %eax
> 	adcl	$0, %edx
> L3:
> 	shrdl	$9, %edx, %eax
> 	sarl	$9, %edx
> 	ret
> 
> Notice the additional compare and test in the 2.96 code.  
> 
> I can't make any sense at all of the RTL.

ix86_expand_branch is emitting the long long compare like this:

         * a < b =>
         *    if (hi(a) < hi(b)) goto true; 
         *    if (hi(a) > hi(b)) goto false;
         *    if (lo(a) < lo(b)) goto true;

In the above case, the third jump can be turned into an uncoditional jump by
cse, which means the first test/jump could be eliminated _if_ it were emitted
directly before the last one (i.e. swap the first two statements above.)

Out of curiousity I made that change:

Index: config/i386/i386.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/i386/i386.c,v
retrieving revision 1.154
diff -c -2 -p -r1.154 i386.c
*** i386.c	2000/04/09 20:26:42	1.154
--- i386.c	2000/04/12 20:16:38
*************** ix86_expand_branch (code, label)
*** 4942,4949 ****
  	ix86_compare_op1 = hi[1];
  
- 	if (code1 != NIL)
- 	  ix86_expand_branch (code1, label);
  	if (code2 != NIL)
  	  ix86_expand_branch (code2, label2);
  
  	ix86_compare_op0 = lo[0];
--- 4942,4949 ----
  	ix86_compare_op1 = hi[1];
  
  	if (code2 != NIL)
  	  ix86_expand_branch (code2, label2);
+ 	if (code1 != NIL)
+ 	  ix86_expand_branch (code1, label);
  
  	ix86_compare_op0 = lo[0];


which unfortunately only yielded:

movl	8(%esp), %edx
	movl	4(%esp), %eax
	testl	%edx, %edx
	js	.L4
	cmpl	$0, %edx
	jmp	.L3
	.p2align 4,,7
.L4:
	addl	$511, %eax
	adcl	$0, %edx
.L3:
	shrdl	$9, %edx, %eax
	sarl	$9, %edx
	ret

where the cmpl is obviously useless, and the jmp could be eliminated if the
initial js were a jns .L3.  The rtl in question after cse is:

(insn 12 11 13 (set (reg:CCNO 17 flags)
        (compare:CCNO (subreg:SI (reg:DI 28) 1)
            (const_int 0 [0x0]))) 5 {cmpsi_ccno_1} (nil)
    (nil))

(jump_insn 13 12 33 (set (pc)
        (if_then_else (lt (reg:CCNO 17 flags)
                (const_int 0 [0x0]))
            (label_ref 18)
            (pc))) 459 {*jcc_1} (nil)
    (nil))

(insn 14 33 15 (set (reg:CC 17 flags)
        (compare:CC (subreg:SI (reg:DI 28) 1)
            (const_int 0 [0x0]))) 6 {cmpsi_1} (nil)
    (nil))

(jump_insn 15 14 34 (set (pc)
        (if_then_else (gt (reg:CC 17 flags)
                (const_int 0 [0x0]))
            (label_ref 20)
            (pc))) 461 {*jcc_3} (nil)
    (nil))

(insn 16 34 17 (set (reg:CCNO 17 flags)
        (compare:CCNO (subreg:SI (reg:DI 28) 0)
            (const_int 0 [0x0]))) 5 {cmpsi_ccno_1} (nil)
    (nil))

(jump_insn 17 16 38 (set (pc)
        (label_ref 20)) -1 (nil)
    (nil))

(barrier 38 17 18)

(code_label 18 38 35 4 "" "" [num uses: 1])


(insn 19 35 20 (parallel[ 
            (set (reg:DI 28)
                (plus:DI (reg:DI 28)
                    (const_int 511 [0x1ff])))
            (clobber (reg:CC 17 flags))
        ] ) 179 {adddi3} (nil)
    (nil))

(code_label 20 19 36 3 "" "" [num uses: 2])


In 2.95.2, insn 16 was deleted by cse, because i386 was a cc0 port and cse_insn
knew cc0 wasn't preserved accross insns.  This allowed the jump pass after cse
to delete jump 15 as well.  In the current compiler, neither is deleted
(because we don't have flow to know that 16 is dead.)

That would be ok, because flow deletes 16 later, and jump2 deletes jump 15.
But we can't delete insn 14 when we delete jump 15, because we can't rely on
the REG_DEAD notes after scheduling (see the comment in delete_computation.)
Since insn 14 can't go, we can't delete jump 17 and invert jump 13.

So all for naught :)  I don't think we can run jump_optimize at the end of
flow2.  Besides, I don't know if my change to i386.c would hurt jump
optimization oppurtunities for other situations.   

It's really a pain that we can't depend on lifetime information throughout the
compiler.  Is there an estimate on how much work it would take to make
the scheduler maintain REG_DEAD notes?

				-Clint