[Bug rtl-optimization/66790] Invalid uninitialized register handling in REE

["derodat at adacore dot com" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66790

--- Comment #4 from Pierre-Marie de Rodat <derodat at adacore dot com> ---
(In reply to Pierre-Marie de Rodat from comment #0)
> Given the "somelabel" code path, I would rather expect DF_REF_CHAIN to hold
> a NULL reference to materialize the lack of initialization in one path. I
> found the DF_LR_IN/DF_LR_OUT macros from df.h: they provide information
> about uninitialized variables but the associated comment says they should
> not be used ("This intolerance should eventually be fixed."). Besides, they
> provide a basic-block-level information whereas we are rather interested in
> instruction-level information in REE.

Having read more thoroughly dataflow code, I saw the DF_LIVE problem claims
that it provides "the logical AND of the IN and OUT sets from the LR problem
and the must-initialized problem", which would be useful: it could provide a
conservative set of registers that *may* be uninitialized (i.e. all registers
that may be uninitialized are included).

So I started looking at the DF_LIVE results, but I got strange results. For
instance, with a very simple C function:

    extern void do_nothing (void);

    int foo (int i, int b) {
      int r;
      if (b) { do_nothing (); r = i; }
      do_nothing ();
      return r & 0xffff;
    }
    /* foobar.c */

â the reference to the register holding "r" at the return statement is present
in the IN set for the corresponding basic block:

     $ gdb --args cc1 -O2 foobar.c
    (gdb) b find_removable_extensions
    (gdb) r
    (gdb) p dump_function_to_file(cfun.decl, stderr, 0)
    [...]
    (note 13 12 14 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
    (call_insn 14 13 15 4 [...])
    (insn 15 14 21 4 (set (reg:SI 0 ax [orig:92 D.1771 ] [92])
            (zero_extend:SI (reg:HI 3 bx [orig:87 r ] [87])))
            foobar.c:13 136 {*zero_extendhisi2}
        (nil))
    [...]
    (gdb) p df_live_top_dump(cfun.cfg.x_basic_block_info.m_vecdata[4], stderr)
    ;; live in 3 [bx] 7 [sp]
    ;; live gen 0 [ax]
    ;; live kill

This looks wrong to me: there is a path from the entry to this basic block that
doesn't cross a definition for the bx register, so it's not must-initialized
and thus should not be present in this set. Digging further, I noticed that in
df_live_confluence_n, we do:

    IN(succ) = OUT(pred) OR IN(succ)

â which, I think, should instead be an AND with respect to the comment: we
want to keep only registers that are must-initialized, so as soon as one *may*
be uninitialized it is out of the result. But this is how it works since the
dataflow merge in 2006â and doing this change unsurprisingly makes the compiler
generate wrong code.

So assuming the LIVE pass actually does not compute must-initialized registers
(but instead what I would call "maybe-initialized registers"), I tried to add
another problem: MIR (Must-Initialized Registers) and to use it in the REE
pass. I'm about to submit the patch on gcc-patches@.