[llvm-dev] analyzePhysReg question

[JF Bastien via llvm-dev]

On Fri, Dec 4, 2015 at 8:55 AM, Smith, Kevin B via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> >-----Original Message-----
> >From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
> >Sanjoy Das via llvm-dev
> >Sent: Thursday, December 03, 2015 11:16 PM
> >To: Quentin Colombet <qcolombet at apple.com>
> >Cc: llvm-dev at lists.llvm.org
> >Subject: Re: [llvm-dev] analyzePhysReg question
> >
> >I think this is related to PR25033:
> >https://llvm.org/bugs/show_bug.cgi?id=25033#c9
>
> Yes, I agree this is very closely related.  The code referenced in that PR
> if (Analysis.Kills || Analysis.Clobbers)
>         // Register killed, so isn't live.
>         return LQR_Dead;
>
> is incorrect I think.  A clobber isn't a full def, and so cannot be
> assumed to be a kill. Some bits from the live-in value may pass through
> into the live-out value.
>
> Looking into the code for anaylzePhysReg, I think the code looks like it
> intends for Defines to truly mean Reg or a super-register of Reg is
> defined, not any overlapping Reg is defined.
> The code looks like this:
>
> bool IsRegOrSuperReg = MOReg == Reg || TRI->isSuperRegister(MOReg, Reg);
> ...
> if (IsRegOrSuperReg) {
>   PRI.Defines = true;     // Reg or a super-register is defined.
>   if (!MO.isDead())
>     AllDefsDead = false;
> }
>
> I think the fundamental bug here is that the operands are swapped when
> passed into isSuperRegister.  The definition of isSuperRegister is
>
> /// \brief Returns true if RegB is a super-register of RegA.
> bool isSuperRegister(unsigned RegA, unsigned RegB) const;
>
> so, it looks to me as if in the call to isSuperRegister, the parameters
> are swapped, and analyzePhysReg should really be asking whether
> the operand Reg (MOReg) is a super register of Reg, and the code should be:
>
> bool IsRegOrSuperReg = MOReg == Reg || TRI->isSuperRegister(Reg, MOReg);


It would be good to check that this maps correctly onto
computeRegisterLiveness: there's a bug in analyzePhysReg and I think other
parts of the code base are slightly wrong or will become slightly wrong as
well :-(
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151204/84a3d31e/attachment.html>