[llvm-commits] [llvm] r73201 - in /llvm/trunk: include/llvm/Target/TargetELFWriterInfo.h lib/CodeGen/ELF.h lib/CodeGen/ELFCodeEmitter.cpp lib/CodeGen/ELFWriter.cpp lib/CodeGen/ELFWriter.h lib/Target/X86/X86ELFWriterInfo.cpp lib/Target/X86/X86ELFWriterInfo.h lib/Target/X86/X86TargetMachine.cpp

[Chris Lattner]

On Jun 11, 2009, at 12:16 PM, Bruno Cardoso Lopes wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=73201&view=rev
> Log:
> Support for ELF Visibility
> Emission for globals, using the correct data sections
> Function alignment can be computed for each target using  
> TargetELFWriterInfo
> Some small fixes

Nice.

> +++ llvm/trunk/include/llvm/Target/TargetELFWriterInfo.h Thu Jun 11  
> 14:16:03 2009
> @@ -14,6 +14,10 @@
> #ifndef LLVM_TARGET_TARGETELFWRITERINFO_H
> #define LLVM_TARGET_TARGETELFWRITERINFO_H
>
> +#include "llvm/Target/TargetData.h"
> +#include "llvm/Target/TargetMachine.h"
> +#include "llvm/Function.h"

Is it possible to forward declare this stuff instead of including it?   
If you move the getFunctionAlignment method out of line it will help.

> +    /// getFunctionAlignment - Returns the alignment for function  
> 'F', targets
> +    /// with different alignment constraints should overload this  
> method
> +    virtual unsigned getFunctionAlignment(const Function *F) const {
> +      const TargetData *TD = TM.getTargetData();
> +      unsigned FnAlign = F->getAlignment();
> +      unsigned TDAlign = TD->getPointerABIAlignment();
> +      unsigned Align = std::max(FnAlign, TDAlign);
> +      assert(!(Align & (Align-1)) && "Alignment is not a power of  
> two!");
> +      return Align;

Bill just committed a patch to move Function Alignment information  
into MachineFunction.  Can you change this to just read that  
information instead of inferring an alignment in the writer-specific  
code?

> +++ llvm/trunk/lib/CodeGen/ELF.h Thu Jun 11 14:16:03 2009
> @@ -21,12 +21,12 @@
> #ifndef CODEGEN_ELF_H
> #define CODEGEN_ELF_H
>
> +#include "llvm/GlobalVariable.h"
> #include "llvm/CodeGen/MachineRelocation.h"
> #include "llvm/Support/DataTypes.h"
> #include <cstring>

Likewise, please try to shrink down #includes where possible.  This  
should not need to include GlobalVariable.h.

Thanks Bruno!

-Chris