[Bug regression/20973] [4.0/4.1 Regression] kdelibs (khtml) miscompiled by reload

["matz at suse dot de" <gcc-bugzilla at gcc dot gnu dot org>]

------- Additional Comments From matz at suse dot de  2005-04-12 17:37 -------
Another mail: 
 
I think the dust settles a bit.  We have this situation in 
.t69.final_cleanup (excerpt from applyRule): 
 
  struct Length D.83927; 
<L2904>:; 
  D.83927.D.21947.l.value = (int) primitiveValue->m_value.num; 
  D.83927.D.21947.l.type = 1; 
  D.83927.D.21947.l.quirk = 0; 
  l = D.83927; 
  apply = 1; 
 
<L2908>:; 
  printf (&"apply %d id %d\n"[0], (int) apply, id); 
  if (apply == 0) goto <L9089>; else goto <L2910>; 
 
The above is the only use of D.83927, so it first is initialized 
memberwise, and then copied as a whole.  This corresponds to these insns 
from .22.lreg (not the full above sequence, but only the interesting 
part).  Reg 4101 is 'l' and is constructed piecewise.  Reg 4197 is 'id'. 
 
;; Start of basic block 1449, registers live: 3 [bx] 6 [bp] 7 [sp] 16 
   [argp] 18 [fpsr] 20 [frame] 3669 3843 4196 4197 
.... 
  (insn:HI 12371 12370 12372 1449 
  /usr/src/packages/BUILD/kdelibs-3.4.0/khtml/misc/khtmllayout.h:49 (parallel 
[ 
            (set (reg/v:SI 4101 [ l ]) 
                (and:SI (reg:SI 3843 [ D.83927 ]) 
                    (const_int -268435456 [0xf0000000]))) 
            (clobber (reg:CC 17 flags)) 
        ]) 200 {*andsi_1} (nil) 
    (expr_list:REG_UNUSED (reg:CC 17 flags) 
        (expr_list:REG_DEAD (reg:SI 3843 [ D.83927 ]) 
            (expr_list:REG_UNUSED (reg:CC 17 flags) 
                (nil))))) 
 
Note how reg 3843 dies here.  Additionally it should be mentioned that 
it's never initialized (it's live up through the first insn), as it 
probably represents the yet-uninitialized part of a bitfield in 
construction.  After the above follows some more bitmanipulation on reg 
4101, setting finally all fields, then the block ends: 
 
;; End of basic block 1449, registers live: 
 3 [bx] 6 [bp] 7 [sp] 16 [argp] 20 [frame] 3668 4101 4196 4197 
 
;; Start of basic block 1450, registers live: 3 [bx] 6 [bp] 7 [sp] 16 
   [argp] 20 [frame] 3668 4101 4196 4197 
 
... 
(insn:HI 12382 12381 12383 1450 
 /usr/src/packages/BUILD/kdelibs-3.4.0/khtml/css/cssstyleselector.cpp:2834 
 (set (mem:SI (plus:SI (reg/f:SI 7 sp) 
                (const_int 8 [0x8])) [0 S4 A32]) 
        (reg/v:SI 4197 [ id ])) 35 {*movsi_1} (nil) 
    (nil)) 
 
So, and this is the setup of the arguments to the printf.  So, we know 
that 4197 is live throughout block 1449, and correctly conflicts with reg 
4101 (see below), which is the only register set here, by the bitmasking 
insn 12371. 
 
Now we look at .23.greg dump, first the interesting conflicts: 
;; 3843 conflicts: 
;; 4101 conflicts: ... 4197 ... 
;; 4197 conflicts: ... 4101 ... 
 
So, all is well.  But then reload goes and breaks it: 
 
... 
Spilling for insn 12371. 
Spilling for insn 12371. 
Reloads for insn # 12371 
Reload 0: reload_in (SI) = (reg:SI 4 si [orig:3843 D.83927 ] [3843]) 
        reload_out (SI) = (reg/v:SI 4101 [ l ]) 
        GENERAL_REGS, RELOAD_OTHER (opnum = 0) 
        reload_in_reg: (reg:SI 4 si [orig:3843 D.83927 ] [3843]) 
        reload_out_reg: (reg/v:SI 4101 [ l ]) 
        reload_reg_rtx: (reg:SI 4 si [orig:3843 D.83927 ] [3843]) 
... 
 
Yes, right, reload needs to fix up the insn to make both registers the 
same and it wants to use $esi as reg for reg3843.  Problem is, it wants 
also to alloc reg4197 to $esi: 
 
... 
3826 in 0  3843 in 4  3844 in 4  3846 in 4  3847 in 1  3912 in 0 
4173 in 5  4180 in 4  4197 in 4  4198 in 5  4199 in 0  4202 in 0 
... 
(4101 is not allocated by the way because reload is able to optimize it 
out, and instead uses 3843 in it's place).  So this then results in this 
insn: 
(insn:HI 12371 12370 43006 
1448 /usr/src/packages/BUILD/kdelibs-3.4.0/khtml/mis 
            (set (reg:SI 4 si [orig:3843 D.83927 ] [3843]) 
                (and:SI (reg:SI 4 si [orig:3843 D.83927 ] [3843]) 
                    (const_int -268435456 [0xf0000000]))) 
            (clobber (reg:CC 17 flags)) 
        ]) 200 {*andsi_1} (nil) 
 
at which point %esi is lost.  I think the problem is, that global-alloc 
uses %esi for 4197 and 3843, which is okay, as 3843 is uninitialized and 
hence doesn't conflict with 4197.  But reload then goes on and actually 
uses this uninitialized register for _setting_.  Basically it transforms 
this insn: 
  rA <- op (rB) 
where rA and rB need to match into 
  rB <- op (rB) 
   [rA <- rB] 
where the latter move is not emitted because rA could be optimized away 
(rA is our 4101). 
 
This introduces a new setting of rB, which is a problem if rB was 
uninitialized.  Normally reload should have generated 
  rA <- rB 
  rA <- op (rA) 
 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20973