[llvm] r180573 - This patch adds the X86FixupLEAs pass, which will reduce instruction

[Nadav Rotem]

Please don't forget the other comments (e.g. remove LiveVariables because it is never available)


On Apr 25, 2013, at 2:07 PM, "Gurd, Preston" <preston.gurd at intel.com> wrote:

> Hello Nadav,
>  
> Thanks, once again, for your attentiveness!
>  
> I had added the documentation just ahead of each function body, using X86PadShortFunction.cpp and X86VZeroUpper.cpp as models.
>  
> I will move the comments for the functions in X86FixupLEAs.cpp to precede the initial declarations.
>  
> Preston
>  
> From: Nadav Rotem [mailto:nrotem at apple.com] 
> Sent: Thursday, April 25, 2013 4:51 PM
> To: Gurd, Preston
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] r180573 - This patch adds the X86FixupLEAs pass, which will reduce instruction
>  
> +
> +    virtual const char *getPassName() const { return "X86 Atom LEA Fixup";}
> +    void  seekLEAFixup(MachineOperand& p, MachineBasicBlock::iterator& I,
> +                      MachineFunction::iterator MFI);
> +    void processInstruction(MachineBasicBlock::iterator& I,
> +                            MachineFunction::iterator MFI);
> +    RegUsageState usesRegister(MachineOperand& p,
> +                               MachineBasicBlock::iterator I);
> +    MachineBasicBlock::iterator searchBackwards(MachineOperand& p,
> +                                                MachineBasicBlock::iterator& I,
> +                                                MachineFunction::iterator MFI);
> +    MachineInstr* postRAConvertToLEA(MachineFunction::iterator &MFI,
> +                                     MachineBasicBlock::iterator &MBBI,
> +                                     LiveVariables *LV) const;
> +
>  
> Can you please document the declarations of these functions ?
> ( see http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments )
>  
> 
> +
> +/// postRAConvertToLEA - if an instruction can be converted to an 
> +/// equivalent LEA, insert the new instruction into the basic block
> +/// and return a pointer to it. Otherwise, return zero.
> +MachineInstr *
> +FixupLEAPass::postRAConvertToLEA(MachineFunction::iterator &MFI,
> +                                 MachineBasicBlock::iterator &MBBI,
> +                                 LiveVariables *LV) const {
>  
>  
> Preston,  LiveVariables are not available in this phase and it is only confusing.  Please remove them from the code. 
>  
> +  LV = getAnalysisIfAvailable<LiveVariables>();  <----- this will always return 0.  Did you ever get a LiveVariable instance ?
>  
> +  // Process all basic blocks.
> +  for (MachineFunction::iterator I = Func.begin(), E = Func.end(); I != E; ++I)
> +    processBasicBlock(Func, I);
> +  DEBUG(dbgs() << "End X86FixupLEAs\n";);
> +
> +  return true;
> +}
>  
>  
> This function always returns true.  You probably only want to return true of the code changed. 
>  
> Can you reduce the size of the test cases ?  Ideally I would like to see a single-basic block function. You only need 5-instructions or so. Why did you include such long tests ?

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130425/9dc30a31/attachment.html>