[PATCH] D37164: [ARM] Fix bug in ARMLoadStoreOptimizer when kill flags are missing.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D37164#854194, @MatzeB wrote:

> In https://reviews.llvm.org/D37164#854053, @gberry wrote:
>
> > In https://reviews.llvm.org/D37164#853040, @efriedma wrote:
> >
> > > If I understand correctly, the original code was checking for the kill flags just to short-circuit the if statement more quickly?
> >
> >
> > That's what it looks like to me: an attempt to reduce the calls to regsOverlap().  Or at least that would make sense for 'BaseKill'.  Checking 'OffKill' I don't get unless 'Base' and 'Off' are somehow known to be related?
>
>
> Indeed checking for the kill flags is odd and removing the checks looks good to me. However there may be something about the offset register: Won't we have the same problem when the first load overwrites the offset register? So maybe we need to check for an overlap with offsetreg as well (and have a bigger problem if base+offset get overridden?)


This whole OffKill/OffUndef handling is confusing. We only ever query the kill and undef flag and don't even bother to query the register itself. We then pass the kill/undef values to `InsertLDR_STR()` which doesn't even use the parameter.

Maybe this is because t2LDRDi8 has an immediate offset and hence no offset register is used, while LDRD/STRD always have consecutive even/odd registers so we should always hit the `OddRegNum > EvenRegNum && OffImm == 0` case earlier in the function in the cases where an offset register is present.

At least smells to me like OffUndef/OffKill uses could be replaced with `assert(!isT2 || MI->getOperand(3).getReg() == 0);`


https://reviews.llvm.org/D37164