[PATCH] D21395: Fix for Bug 28144

[Vivek Pandya via llvm-commits]

vivekvpandya added a comment.

@mehdi_amini , at MatzeB 
Here is my analysis:

  BitVector UsedPhysRegMask = MRI->getUsedPhysRegsMask();
    for (unsigned i = 0; i < RegMaskSize; ++i)
      RegMask[i] &= ~UsedPhysRegMask[i];

The above code is buggy because UsedPhysRegMask is indexed by register no where 
as RegMask is vector integers in which each bit represenet status of one register.
I am sorry for that, I assumed that in LLVM all regmask are following same representation.

  // Code snippet 1 (CS1)
    for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg)
      if (!MRI->reg_nodbg_empty(PReg) && MRI->isPhysRegUsed(PReg))
        markRegClobbered(TRI, &RegMask[0], PReg);



  // Code snippet 2 (CS2)
    for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg)
      if (MRI->isPhysRegUsed(PReg))
        markRegClobbered(TRI, &RegMask[0], PReg);

Now actaully CS2 is sufficient and correct code for calculating regmask. And CS1
is having a very subtle bug which caused incorrect regmask (such as not marking
R11 as clobbered). Now why CS2 is correct and sufficient (I think @MatzeB has
pointed this fact earlier) it is because MRI::isPhysRegUsed actaully uses
UsedPhysRegMask so no need to consider that details again and MRI::isPhysRegUsed
also uses reg_nodbg_empty.

Now what is the problem with CS1, due to !MRI->reg_nodbg_empty(PReg) condition
fails when MF is not using PReg and MRI->isPhysRegUsed(PReg) would never execute.
thus generating wrong regmask. So why it happens so. Consider following definition
of reg_nodbg_empty and reg_nodbg_begin, it uses getRegUseDefListHead and if given
register is not used in side a MF then it will return true (I think it should not
Isn't reg_nodbg_empty return false if Reg is not used in MF at all?)
and due to that !MRI->reg_nodbg_empty(PReg) will be false.

  /// reg_nodbg_empty - Return true if the only instructions using or defining
  /// Reg are Debug instructions.
    bool reg_nodbg_empty(unsigned RegNo) const {
       return reg_nodbg_begin(RegNo) == reg_nodbg_end();
    }
  
    reg_nodbg_iterator reg_nodbg_begin(unsigned RegNo) const {
       return reg_nodbg_iterator(getRegUseDefListHead(RegNo));
     }

What are your thoughts? Have I missed any thing?


http://reviews.llvm.org/D21395