Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

Steven Bosscher stevenb.gcc@gmail.com
Mon Oct 15 13:19:00 GMT 2012


On Mon, Oct 15, 2012 at 1:49 PM, Paolo Bonzini wrote:
>> I strongly disagree with this approach though. It destroys information
>> that is correct, that we had before DF_RD_PRUNE_DEAD_DEFS, that we can
>> update, and that helps with optimization. With renaming these notes
>> are valid, and do not refer to dead regs
>
> I agree it is bad.  But I do not understand the last sentence: I
> suppose you mean that _without_ renaming these notes are valid, on the
> other hand it is normal that some of the notes will be dropped if you
> shorten live ranges.

OK, I now got so confused that I looked into this a bit deeper.

The situation is something like the following just after unrolling,
but before web:

   33 r72:DI=[`rowArray']
   34 {r103:DI=r72:DI+0x18;clobber flags:CC;}
   ...
   45 flags:CCNO=cmp([r72:DI+0x20],0)
      REG_DEAD: r72:DI
;; diamond shape region follows, joining up again in bb 9:
   79 r72:DI=r103:DI
      REG_EQUAL: r72:DI+0x18

On entry to bb9, r72 is not in LR_IN, so after loop unrolling this
note is already invalid if we say that a note should not refer to a
dead register.


But the register dies much earlier. The first place where insn 79
appears is in the .169r.pre dump:

;; basic block 8, loop depth 1, count 0, freq 9100, maybe hot
;;  prev block 7, next block 9, flags: (REACHABLE, RTL, MODIFIED)
;;  pred:       7
;;              6
;; bb 8 artificial_defs: { }
;; bb 8 artificial_uses: { u43(6){ }u44(7){ }u45(16){ }u46(20){ }}
;; lr  in        6 [bp] 7 [sp] 16 [argp] 20 [frame] 72 82 85 87
;; lr  use       6 [bp] 7 [sp] 16 [argp] 20 [frame] 72 87
;; lr  def       17 [flags] 72
;; live  in      6 [bp] 7 [sp] 16 [argp] 20 [frame] 72 82 85 87
;; live  gen     17 [flags] 72
;; live  kill    17 [flags]
L49:
   50 NOTE_INSN_BASIC_BLOCK
   79 r72:DI=r103:DI
      REG_EQUAL: r72:DI+0x18

Here, r72 is still in LR_IN so the note is valid.


Then in .171r.cprop2:

;; basic block 8, loop depth 1, count 0, freq 9100, maybe hot
;;  prev block 7, next block 9, flags: (REACHABLE, RTL, MODIFIED)
;;  pred:       7
;;              6
;; bb 8 artificial_defs: { }
;; bb 8 artificial_uses: { u43(6){ }u44(7){ }u45(16){ }u46(20){ }}
;; lr  in        6 [bp] 7 [sp] 16 [argp] 20 [frame] 82 85 87 103
;; lr  use       6 [bp] 7 [sp] 16 [argp] 20 [frame] 87 103
;; lr  def       17 [flags] 72
;; live  in      6 [bp] 7 [sp] 16 [argp] 20 [frame] 82 85 87 103
;; live  gen     17 [flags] 72
;; live  kill
L49:
   50 NOTE_INSN_BASIC_BLOCK
   79 r72:DI=r103:DI
      REG_EQUAL: r72:DI+0x18

So already after CPROP2, the REG_EQUAL note is invalid if we require
that they only refer to live registers. This all happens well before
any pass uses the DF_RD problem, so this is a pre-existing problem if
we consider this kind of REG_EQUAL note to be invalid.


> Without removing all of the notes you can do something like this:
>
> - drop the deferred rescanning from web.c.  Instead, make replace_ref
> return a bool and call df_insn_rescan manually from web_main.
>
> - attribute new registers to webs in a separate pass that happens
> before rewriting, and compute a special version of LR_IN/LR_OUT that
> uses the rewritten registers.
>
> - process instructions in reverse order; before starting the visit of
> a basic block, initialize the local LR bitmap with the rewritten
> LR_OUT of the previous step
>
> - after rewriting and scanning each statement, simulate liveness using
> the new defs and uses.
>
> - after rewriting each statement, look for EQ_USES referring to
> registers that are dead just before the statement, and delete
> REG_EQUAL notes if this is the case

I think I've shown above that we're all looking at the wrong pass...

Ciao!
Steven



More information about the Gcc-patches mailing list