[llvm-commits] [llvm] r47366 - /llvm/trunk/lib/CodeGen/LiveVariables.cpp

[Evan Cheng]

On Feb 20, 2008, at 1:15 AM, Bill Wendling wrote:

>
> +  // Now reset the use information for the sub-registers.
>   for (const unsigned *SubRegs = RegInfo->getSubRegisters(Reg);
>        unsigned SubReg = *SubRegs; ++SubRegs) {
> +    // FIXME: Should we do: "PhysRegPartUse[SubReg] = NULL;" here?

Yes, looks like a bug. Please make that change and do some testing.

>
>     PhysRegInfo[SubReg] = MI;
>     PhysRegUsed[SubReg] = true;
>   }
>
>   for (const unsigned *SuperRegs = RegInfo->getSuperRegisters(Reg);
>        unsigned SuperReg = *SuperRegs; ++SuperRegs) {
> -    // Remember the partial use of this superreg if it was  
> previously defined.
> +    // Remember the partial use of this super-register if it was  
> previously
> +    // defined.
>     bool HasPrevDef = PhysRegInfo[SuperReg] != NULL;
> -    if (!HasPrevDef) {
> +
> +    if (!HasPrevDef)
> +      // FIXME: This only goes back one level of super-registers.  
> It might miss
> +      // some.

No need to go up more levels. A def of a register also sets its sub- 
registers (so if PhysRegInfo[SuperReg] is NULL, it means SuperReg's  
super registers are not previously defined).

Evan

>
>       for (const unsigned *SSRegs = RegInfo- 
> >getSuperRegisters(SuperReg);
> -           unsigned SSReg = *SSRegs; ++SSRegs) {
> +           unsigned SSReg = *SSRegs; ++SSRegs)
>         if (PhysRegInfo[SSReg] != NULL) {
>           HasPrevDef = true;
>           break;
>         }
> -      }
> -    }
> +
>     if (HasPrevDef) {
>       PhysRegInfo[SuperReg] = MI;
>       PhysRegPartUse[SuperReg] = MI;
> @@ -413,7 +430,7 @@
>   std::fill(PhysRegUsed, PhysRegUsed + NumRegs, false);
>   std::fill(PhysRegPartUse, PhysRegPartUse + NumRegs,  
> (MachineInstr*)0);
>
> -  /// Get some space for a respectable number of registers...
> +  /// Get some space for a respectable number of registers.
>   VirtRegInfo.resize(64);
>
>   analyzePHINodes(mf);
> @@ -422,9 +439,9 @@
>   // function.  This guarantees that we will see the definition of a  
> virtual
>   // register before its uses due to dominance properties of SSA  
> (except for PHI
>   // nodes, which are treated as a special case).
> -  //
>   MachineBasicBlock *Entry = MF->begin();
>   SmallPtrSet<MachineBasicBlock*,16> Visited;
> +
>   for (df_ext_iterator<MachineBasicBlock*,  
> SmallPtrSet<MachineBasicBlock*,16> >
>          DFI = df_ext_begin(Entry, Visited), E = df_ext_end(Entry,  
> Visited);
>        DFI != E; ++DFI) {
> @@ -451,7 +468,7 @@
>       if (MI->getOpcode() == TargetInstrInfo::PHI)
>         NumOperandsToProcess = 1;
>
> -      // Process all uses...
> +      // Process all uses.
>       for (unsigned i = 0; i != NumOperandsToProcess; ++i) {
>         const MachineOperand &MO = MI->getOperand(i);
>
> @@ -466,7 +483,7 @@
>         }
>       }
>
> -      // Process all defs...
> +      // Process all defs.
>       for (unsigned i = 0; i != NumOperandsToProcess; ++i) {
>         const MachineOperand &MO = MI->getOperand(i);
>
> @@ -501,8 +518,8 @@
>                                 MBB);
>     }
>
> -    // Finally, if the last instruction in the block is a return,  
> make sure to mark
> -    // it as using all of the live-out values in the function.
> +    // Finally, if the last instruction in the block is a return,  
> make sure to
> +    // mark it as using all of the live-out values in the function.
>     if (!MBB->empty() && MBB->back().getDesc().isReturn()) {
>       MachineInstr *Ret = &MBB->back();
>
> @@ -520,7 +537,7 @@
>     }
>
>     // Loop over PhysRegInfo, killing any registers that are  
> available at the
> -    // end of the basic block.  This also resets the PhysRegInfo map.
> +    // end of the basic block. This also resets the PhysRegInfo map.
>     for (unsigned i = 0; i != NumRegs; ++i)
>       if (PhysRegInfo[i])
>         HandlePhysRegDef(i, 0);
> @@ -536,7 +553,6 @@
>
>   // Convert and transfer the dead / killed information we have  
> gathered into
>   // VirtRegInfo onto MI's.
> -  //
>   for (unsigned i = 0, e1 = VirtRegInfo.size(); i != e1; ++i)
>     for (unsigned j = 0, e2 = VirtRegInfo[i].Kills.size(); j != e2; + 
> +j)
>       if (VirtRegInfo[i].Kills[j] ==
> @@ -568,10 +584,10 @@
>   return false;
> }
>
> -/// instructionChanged - When the address of an instruction  
> changes, this
> -/// method should be called so that live variables can update its  
> internal
> -/// data structures.  This removes the records for OldMI,  
> transfering them to
> -/// the records for NewMI.
> +/// instructionChanged - When the address of an instruction  
> changes, this method
> +/// should be called so that live variables can update its internal  
> data
> +/// structures.  This removes the records for OldMI, transfering  
> them to the
> +/// records for NewMI.
> void LiveVariables::instructionChanged(MachineInstr *OldMI,
>                                        MachineInstr *NewMI) {
>   // If the instruction defines any virtual registers, update the  
> VarInfo,
> @@ -632,9 +648,8 @@
> }
>
> /// analyzePHINodes - Gather information about the PHI nodes in  
> here. In
> -/// particular, we want to map the variable information of a virtual
> -/// register which is used in a PHI node. We map that to the BB the  
> vreg is
> -/// coming from.
> +/// particular, we want to map the variable information of a  
> virtual register
> +/// which is used in a PHI node. We map that to the BB the vreg is  
> coming from.
> ///
> void LiveVariables::analyzePHINodes(const MachineFunction& Fn) {
>   for (MachineFunction::const_iterator I = Fn.begin(), E = Fn.end();
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits