[PR81611] improve auto-inc
Jeff Law
law@redhat.com
Wed Feb 28 02:39:00 GMT 2018
On 02/27/2018 04:18 PM, Alexandre Oliva wrote:
> On Feb 14, 2018, Jeff Law <law@redhat.com> wrote:
>
>>> + regno = REGNO (inc_insn.reg0);
>>> + int luid = DF_INSN_LUID (mem_insn.insn);
>>> + mem_insn.insn = get_next_ref (regno, bb, reg_next_use);
>> So I think a comment is warranted right as we enter the TRUE arm.
>
>> At that point INC_INSN is an inc/dec. But MEM_INSN is not necessarily a
>> memory reference. It could be a memory reference, it could be a copy,
>> it could be something completely different (it's just the next insn that
>> references the result of the increment). In the case we care about we
>> want it to be a copy of INC_INSN's REG_RES back to REG0.
>
>> ISTM that verifying MEM_INSN is a reg->reg copy (reg_res -> reg0) before
>> we call get_next_ref for reg0 is advisable and probably good from a
>> compile-time standpoint by avoiding calls into find_address.
>
> But we don't need it to be a copy. The transformation is just as
> legitimate if the regs go independent ways after that point. We have
> reg_res set to reg0+reg1, and then a use of reg0 in a MEM before any
> other use of reg_res. We turn that into a copy of reg0 to reg_res, and
> the MEM addr into a post_add of reg_res with reg1 (possibly a post_inc),
> so that the MEM dereferences reg_res while it's still equal to reg0, and
> after the MEM, reg_res becomes reg0+reg1, as it should for any
> subsequent uses, and reg0 is unmodified. Whether or not a subsequent
> copy from reg_res to reg0 is to be found won't make the transformation
> any more or less legitimate.
>
>> After we call get_next_ref to get the next reference of the source of
>> the increment, then we're hoping to find a memory reference that uses
>> REG0. But it's not guaranteed it's a memory reference insn.
>
> Yeah, find_address will determine if it contains any of the MEM patterns
> we might be interested in, but it could be anything whatsoever. The MEM
> pattern might appear virtually anywhere in the insn.
>
>> I was having an awful time understanding how this code could work from
>> the comments until I put it under a debugger and got a sense of the
>> state as we entered that IF block. Then it was much clearer :-)
>
> Sorry, I realize the comments were written based on a lot of context
> about the overall behavior of the pass, that I had learned while trying
> to figure it out. At the risk of making it redundant, I've expanded the
> comments, and added further tests that won't affect current behavior in
> any significant way, but that might speed things up a bit and will save
> us trouble should find_address be extended to catch additional patterns.
>
>
>> I believe Georg had other testcases in subsequent comments in the BZ,
>> but I don't believe they were flagged as regressions.
>
> However, with the testcases I realized the incremented register could
> still be live, even if we didn't find a subsequent use for it.
> Adjusting for that made those testcases use post_inc too.
>
> Here's the improved patch, regstrapped on aarch64-, ppc64-, and
> ppc64el-linux-gnu. Ok to install?
>
>
> [PR81611] turn inc-and-use-of-dead-orig into auto-inc
>
> When the addressing modes available on the machine don't allow offsets
> in addresses, odds are that post-increments will be represented in
> trees and RTL as:
>
> y <= x + 1
> ... *(x) ...
> x <= y
>
> so deal with it by turning such RTL as:
>
> (set y (plus x n))
> ... (mem x) ...
>
> without intervening uses of y into
>
> (set y x)
> ... (mem (post_add y n)) ...
>
> so as to create auto-inc addresses that we'd otherwise miss.
>
>
> for gcc/ChangeLog
>
> PR rtl-optimization/81611
> * auto-inc-dec.c (attempt_change): Move dead note from
> mem_insn if it's the next use of regno
> (find_address): Take address use of reg holding
> non-incremented value. Add parm to limit search to the named
> reg only.
> (merge_in_block): Attempt to use a mem insn that is the next
> use of the original regno.
OK. Thanks!
Jeff
More information about the Gcc-patches
mailing list