Re: Fix PR rtl-optimization/33822

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

> So, going back to the original REG_OFFSET question, I think it would make
> more sense for the REG_OFFSET of (reg:M R) to be the offset of the first
> byte in the M-mode value.  That's what MEM_OFFSET is -- more later about
> why we need negative offsets there too -- and var-tracking.c fundamentally
> assumes that memory offsets and register offsets are comparable.

This makes sense, I think.

> Luckily, the code that sets REG_OFFSET is localised to emit-rtl.c, with
> the rest of the compiler using higher-level interfaces.  (Thanks Jan!)
> So it's easy to enforce a consistent representation.  Also, the problem
> identified in PR 14084 resurfaces if you revert the associated patch,
> so it's easy to verify that this alternative approach fixes it.
>
> So, the patch below:
>
>   - Removes the 14084 code.

You need to update the head comment of the function.

>   - Adds a byte_lowpart_offset function to go alongside
>     subreg_lowpart_offset.  Like subreg_lowpart_offset, this new function
>     copes with identity lowparts, true lowparts and paradoxical lowparts.
>     The offset can be negative for paradoxical lowparts on big-endian
>     systems.

I think that we should make it explicit (in the head comment of the functions 
for example) that subreg_lowpart_offset is meant for SUBREG_BYTE and the new 
byte_lowpart_offset is meant for REG_OFFSET.

>   - Makes sure that an M-mode REG for an N-mode decl starts off
>     with a REG_OFFSET of "byte_lowpart (M, N)".

Right, so don't we need to do something for gen_rtx_REG_offset and 
gen_reg_rtx_offset, at least document what type of offsets they expect?

You have updated one caller (var_lowpart), what about the others?  In 
particular, it seems to me that simplify_subreg already computes the 
byte_lowpart_offset manually before invoking gen_rtx_REG_offset.

> And that's the end of this particular story as far as recording
> the offsets goes.  There are also some problems in the way
> var-tracking uses them:
>
>   - My original patch for paradoxical lowparts wasn't aggressive enough.
>     We also need to cope with promoted variables, such as 32-bit variables
>     stored in 64-bit pseudo registers.
>
>   - We need to do the same when setting up the locations of
>     promoted parameters.
>
>   - We need to do the same when a 64-bit promoted variable is spilled
>     to the stack.  We currently emit a memory location for the start
>     of the spill slot, which is wrong on big-endian targets when the
>     slot is padded.
>
>   - We currently assume that a store to a register that holds VAR+OFFSET
>     does not clobber [VAR+0, VAR+OFFSET).  That's wrong when the register
>     is big enough to hold the whole of VAR.  E.g. it's perfectly possible
>     for a target to store to (reg:SI R) and then reference the value using
>     (reg:DI R), where (reg:DI R) is a single register.
>
> Again, these are all problems that lead to wrong debug info.  And luckily,
> they're not really separate problems at all.  The idea is that if we see
> a reference to VAR with an offset consistent with a lowpart of VAR and
> either:
>
>   1) that lowpart is paradoxical; or
>   2) we're storing into a register that can hold the whole of VAR
>
> then we should consider this to be an access to the whole of VAR.
>
> After those changes, we can enforce the [0, MAX_VAR_PARTS) range,
> thus avoiding the problem that sparked my original reply.
> (Rejecting negative offsets has other benefits too; see later.)

If you have eliminated all but one invocation of offset_valid_for_tracked_p, 
just integrate the function into its only caller.

> Now I realise that the patch might be a bit daunting, but I think it's
> actually very localised, and it's all bug fixes.  I really do think it's
> stage 3 material.

OK, it should indeed be safe.

> However, because it isn't exactly a one-liner either, I've tried to go an
> extra mile as far as testing goes. 

That's an understatement, more like a full marathon in my opinion. :-)

Thanks for conduction such an extensive testing.

-- 
Eric Botcazou