> On Dec 3, 2015, at 5:36 PM, Quentin Colombet via llvm-dev <llvm-dev at lists.llvm.org> wrote: > >> >> On Dec 3, 2015, at 5:11 PM, Smith, Kevin B <kevin.b.smith at intel.com <mailto:kevin.b.smith at intel.com>> wrote: >> >> >> >>> -----Original Message----- >>> From: Quentin Colombet [mailto:qcolombet at apple.com <mailto:qcolombet at apple.com>] >>> Sent: Thursday, December 03, 2015 4:43 PM >>> To: Smith, Kevin B <kevin.b.smith at intel.com <mailto:kevin.b.smith at intel.com>> >>> Cc: llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >>> Subject: Re: [llvm-dev] analyzePhysReg question >>> >>> >>>> On Dec 3, 2015, at 4:35 PM, Smith, Kevin B via llvm-dev <llvm- >>> dev at lists.llvm.org <mailto:dev at lists.llvm.org>> wrote: >>>> >>>> I am looking at results from analyzePhysReg, and am getting results a little >>> different than I expected for x86. >>>> >>>> The call to this is coming from this code in >>> llvm::MachineBasicBlock::computeRegisterLiveness >>>> 1163 MachineOperandIteratorBase::PhysRegInfo Analysis >>>> 1164 ConstMIOperands(I).analyzePhysReg(Reg, TRI); >>>> >>>> The instruction I being analyzed is: >>>> %BX<def> = MOV16rm %EDI, 2, %ECX, 0, %noreg; >>> mem:LD2[%arrayidx98](tbaa=!18) >>>> >>>> and the Reg being passed in is 21, which is EBX. The result I get back for >>> is: >>>> >>>> Analysis: {Clobbers = true, Defines = true, Reads = false, ReadsOverlap >>> false, >>>> DefinesDead = false, Kills = false} >>>> >>>> It seems based on the comment in the definition of PhysRegInfo.Defines, >>> that Defines should only be true if Reg or a super-register of Reg is >>>> defined. BX is not a super-register of EBX, so it seemed like Defines >>> should be false here, while Clobbers is correct as true. >>> >>> I believe the documentation is wrong here. >>> EBX is partly defined, so to me Defines == true is conservatively correct >>> here. >>> My guess, but I haven’t checked the code, is that super-register should be >>> understood as any alias of Reg. >>> >>> Worth checking. >>> >>> Q. >> >> I agree EBX is partly defined, but I think that is what Clobbers is for. > > I think Clobbers is when there is a RegMask. > Basically, if that helps, > Clobber: The register cannot live through. > Define: (Part of) the register is define by this instruction.Put differently, you can use a register as a definition of an instruction when it is clobbered but not defined.> >> The interface for isSuperRegister >> certainly makes a pretty clear distinction between something being a superRegister and something being an >> overlapping register. What do you think I should be checking to understand the assumptions/expectations better? > > The code :). > >> >>> >>>> >>>> I wanted to be sure that I wasn't missing something about the interface >>> definition/expectation. >>>> >>>> Thanks, >>>> Kevin Smith >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151203/5b8e8663/attachment.html>
I think this is related to PR25033: https://llvm.org/bugs/show_bug.cgi?id=25033#c9 -- Sanjoy On Thu, Dec 3, 2015 at 5:39 PM, Quentin Colombet via llvm-dev <llvm-dev at lists.llvm.org> wrote:> > On Dec 3, 2015, at 5:36 PM, Quentin Colombet via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > > On Dec 3, 2015, at 5:11 PM, Smith, Kevin B <kevin.b.smith at intel.com> wrote: > > > > -----Original Message----- > From: Quentin Colombet [mailto:qcolombet at apple.com] > Sent: Thursday, December 03, 2015 4:43 PM > To: Smith, Kevin B <kevin.b.smith at intel.com> > Cc: llvm-dev at lists.llvm.org > Subject: Re: [llvm-dev] analyzePhysReg question > > > On Dec 3, 2015, at 4:35 PM, Smith, Kevin B via llvm-dev <llvm- > > dev at lists.llvm.org> wrote: > > > I am looking at results from analyzePhysReg, and am getting results a little > > different than I expected for x86. > > > The call to this is coming from this code in > > llvm::MachineBasicBlock::computeRegisterLiveness > > 1163 MachineOperandIteratorBase::PhysRegInfo Analysis > 1164 ConstMIOperands(I).analyzePhysReg(Reg, TRI); > > The instruction I being analyzed is: > %BX<def> = MOV16rm %EDI, 2, %ECX, 0, %noreg; > > mem:LD2[%arrayidx98](tbaa=!18) > > > and the Reg being passed in is 21, which is EBX. The result I get back for > > is: > > > Analysis: {Clobbers = true, Defines = true, Reads = false, ReadsOverlap > > false, > > DefinesDead = false, Kills = false} > > It seems based on the comment in the definition of PhysRegInfo.Defines, > > that Defines should only be true if Reg or a super-register of Reg is > > defined. BX is not a super-register of EBX, so it seemed like Defines > > should be false here, while Clobbers is correct as true. > > I believe the documentation is wrong here. > EBX is partly defined, so to me Defines == true is conservatively correct > here. > My guess, but I haven’t checked the code, is that super-register should be > understood as any alias of Reg. > > Worth checking. > > Q. > > > I agree EBX is partly defined, but I think that is what Clobbers is for. > > > I think Clobbers is when there is a RegMask. > Basically, if that helps, > Clobber: The register cannot live through. > Define: (Part of) the register is define by this instruction. > > > Put differently, you can use a register as a definition of an instruction > when it is clobbered but not defined. > > > The interface for isSuperRegister > certainly makes a pretty clear distinction between something being a > superRegister and something being an > overlapping register. What do you think I should be checking to understand > the assumptions/expectations better? > > > The code :). > > > > > I wanted to be sure that I wasn't missing something about the interface > > definition/expectation. > > > Thanks, > Kevin Smith > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- Sanjoy Das http://playingwithpointers.com
>-----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#c9Yes, 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);> >-- Sanjoy > >On Thu, Dec 3, 2015 at 5:39 PM, Quentin Colombet via llvm-dev ><llvm-dev at lists.llvm.org> wrote: >> >> On Dec 3, 2015, at 5:36 PM, Quentin Colombet via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >> >> >> On Dec 3, 2015, at 5:11 PM, Smith, Kevin B <kevin.b.smith at intel.com> >wrote: >> >> >> >> -----Original Message----- >> From: Quentin Colombet [mailto:qcolombet at apple.com] >> Sent: Thursday, December 03, 2015 4:43 PM >> To: Smith, Kevin B <kevin.b.smith at intel.com> >> Cc: llvm-dev at lists.llvm.org >> Subject: Re: [llvm-dev] analyzePhysReg question >> >> >> On Dec 3, 2015, at 4:35 PM, Smith, Kevin B via llvm-dev <llvm- >> >> dev at lists.llvm.org> wrote: >> >> >> I am looking at results from analyzePhysReg, and am getting results a little >> >> different than I expected for x86. >> >> >> The call to this is coming from this code in >> >> llvm::MachineBasicBlock::computeRegisterLiveness >> >> 1163 MachineOperandIteratorBase::PhysRegInfo Analysis >> 1164 ConstMIOperands(I).analyzePhysReg(Reg, TRI); >> >> The instruction I being analyzed is: >> %BX<def> = MOV16rm %EDI, 2, %ECX, 0, %noreg; >> >> mem:LD2[%arrayidx98](tbaa=!18) >> >> >> and the Reg being passed in is 21, which is EBX. The result I get back for >> >> is: >> >> >> Analysis: {Clobbers = true, Defines = true, Reads = false, ReadsOverlap >> >> false, >> >> DefinesDead = false, Kills = false} >> >> It seems based on the comment in the definition of PhysRegInfo.Defines, >> >> that Defines should only be true if Reg or a super-register of Reg is >> >> defined. BX is not a super-register of EBX, so it seemed like Defines >> >> should be false here, while Clobbers is correct as true. >> >> I believe the documentation is wrong here. >> EBX is partly defined, so to me Defines == true is conservatively correct >> here. >> My guess, but I haven’t checked the code, is that super-register should be >> understood as any alias of Reg. >> >> Worth checking. >> >> Q. >> >> >> I agree EBX is partly defined, but I think that is what Clobbers is for. >> >> >> I think Clobbers is when there is a RegMask. >> Basically, if that helps, >> Clobber: The register cannot live through. >> Define: (Part of) the register is define by this instruction. >> >> >> Put differently, you can use a register as a definition of an instruction >> when it is clobbered but not defined. >> >> >> The interface for isSuperRegister >> certainly makes a pretty clear distinction between something being a >> superRegister and something being an >> overlapping register. What do you think I should be checking to >understand >> the assumptions/expectations better? >> >> >> The code :). >> >> >> >> >> I wanted to be sure that I wasn't missing something about the interface >> >> definition/expectation. >> >> >> Thanks, >> Kevin Smith >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > > >-- >Sanjoy Das >http://playingwithpointers.com >_______________________________________________ >LLVM Developers mailing list >llvm-dev at lists.llvm.org >http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev