[llvm-commits] [llvm] r68032 - in /llvm/trunk: include/llvm/Target/ELFTargetAsmInfo.h lib/Target/DarwinTargetAsmInfo.cpp lib/Target/ELFTargetAsmInfo.cpp lib/Target/TargetAsmInfo.cpp lib/Target/X86/X86TargetAsmInfo.cpp

[Frits van Bommel]

Anton Korobeynikov wrote:
> Modified: llvm/trunk/lib/Target/ELFTargetAsmInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ELFTargetAsmInfo.cpp?rev=68032&r1=68031&r2=68032&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/Target/ELFTargetAsmInfo.cpp (original)
> +++ llvm/trunk/lib/Target/ELFTargetAsmInfo.cpp Mon Mar 30 10:27:43 2009
> @@ -44,6 +44,30 @@
>                                            SectionFlags::Writeable);
>  }
>  
> +SectionKind::Kind
> +ELFTargetAsmInfo::SectionKindForGlobal(const GlobalValue *GV) const {
> +  SectionKind::Kind Kind = TargetAsmInfo::SectionKindForGlobal(GV);
> +
> +  if (Kind != SectionKind::Data)
> +    return Kind;
> +
> +  // Decide, whether we need data.rel stuff
> +  const GlobalVariable* GVar = dyn_cast<GlobalVariable>(GV);
> +  if (GVar->hasInitializer()) {
> +    Constant *C = GVar->getInitializer();
> +    bool isConstant = GVar->isConstant();
> +    unsigned Reloc = RelocBehaviour();
> +    if (Reloc != Reloc::None && C->ContainsRelocations(Reloc))

> +      return (C->ContainsRelocations(Reloc::Local) ?
> +              (isConstant ?
> +               SectionKind::DataRelROLocal : SectionKind::DataRelLocal) :
> +              (isConstant ?
> +               SectionKind::DataRelRO : SectionKind::DataRel));

Doesn't this last bit put data with both global and local relocations in 
the section supposedly reserved for data with *only* local relocations? 
(According to the comments you added in the previous commit)

So shouldn't this be something like:
----
+      return (C->ContainsRelocations(Reloc::Global) ?
+              (isConstant ?
+               SectionKind::DataRelRO : SectionKind::DataRel) :
+              (isConstant ?
+               SectionKind::DataRelROLocal : SectionKind::DataRelLocal));
----
instead?

> +  }
> +
> +  return Kind;
> +}
> +
>  const Section*
>  ELFTargetAsmInfo::SelectSectionForGlobal(const GlobalValue *GV) const {
>    SectionKind::Kind Kind = SectionKindForGlobal(GV);
>