[llvm] r224935 - Refactor duplicated code.

Rafael EspĂ­ndola rafael.espindola at gmail.com
Wed Jan 7 18:01:18 PST 2015


I was able to reproduce this with a small objc project and committed a
testcase in r225385.

A revised patch is attached. Could you please give it a try?

On 6 January 2015 at 18:12, Lang Hames <lhames at gmail.com> wrote:
> Hi Rafael,
>
> This caused some Darwin ObjC++/AArch64 test cases to fail during object
> emission. I've reverted this in r225311 to get the bots green while I try to
> reduce a test case. As soon as I have one I'll file a PR and attach it.
> Sorry for the hassle!
>
> Cheers,
> Lang.
>
> On Mon, Dec 29, 2014 at 7:18 AM, Rafael Espindola
> <rafael.espindola at gmail.com> wrote:
>>
>> Author: rafael
>> Date: Mon Dec 29 09:18:31 2014
>> New Revision: 224935
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=224935&view=rev
>> Log:
>> Refactor duplicated code.
>>
>> No intended functionality change.
>>
>> Modified:
>>     llvm/trunk/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
>>     llvm/trunk/include/llvm/MC/MCAsmBackend.h
>>     llvm/trunk/include/llvm/MC/MCAsmInfo.h
>>     llvm/trunk/include/llvm/MC/MCAsmInfoDarwin.h
>>     llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h
>>     llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
>>     llvm/trunk/lib/MC/MCAsmInfo.cpp
>>     llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp
>>     llvm/trunk/lib/MC/MCAssembler.cpp
>>     llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.h
>>     llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h
>>     llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>>     llvm/trunk/lib/Target/TargetMachine.cpp
>>     llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
>>     llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
>> (original)
>> +++ llvm/trunk/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h Mon Dec
>> 29 09:18:31 2014
>> @@ -89,8 +89,6 @@ public:
>>                         ArrayRef<Module::ModuleFlagEntry> ModuleFlags,
>>                         Mangler &Mang, const TargetMachine &TM) const
>> override;
>>
>> -  bool isSectionAtomizableBySymbols(const MCSection &Section) const
>> override;
>> -
>>    const MCSection *
>>      SelectSectionForGlobal(const GlobalValue *GV,
>>                             SectionKind Kind, Mangler &Mang,
>>
>> Modified: llvm/trunk/include/llvm/MC/MCAsmBackend.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmBackend.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/MC/MCAsmBackend.h (original)
>> +++ llvm/trunk/include/llvm/MC/MCAsmBackend.h Mon Dec 29 09:18:31 2014
>> @@ -67,14 +67,6 @@ public:
>>      return false;
>>    }
>>
>> -  /// isSectionAtomizable - Check whether the given section can be split
>> into
>> -  /// atoms.
>> -  ///
>> -  /// \see MCAssembler::isSymbolLinkerVisible().
>> -  virtual bool isSectionAtomizable(const MCSection &Section) const {
>> -    return true;
>> -  }
>> -
>>    /// @name Target Fixup Interfaces
>>    /// @{
>>
>>
>> Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
>> +++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Mon Dec 29 09:18:31 2014
>> @@ -382,6 +382,12 @@ public:
>>      return nullptr;
>>    }
>>
>> +  /// \brief True if the section is atomized using the symbols in it.
>> +  /// This is false if the section is not atomized at all (most ELF
>> sections) or
>> +  /// if it is atomized based on its contents (MachO' __TEXT,__cstring
>> for
>> +  /// example).
>> +  virtual bool isSectionAtomizableBySymbols(const MCSection &Section)
>> const;
>> +
>>    virtual const MCExpr *getExprForPersonalitySymbol(const MCSymbol *Sym,
>>                                                      unsigned Encoding,
>>                                                      MCStreamer &Streamer)
>> const;
>>
>> Modified: llvm/trunk/include/llvm/MC/MCAsmInfoDarwin.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfoDarwin.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/MC/MCAsmInfoDarwin.h (original)
>> +++ llvm/trunk/include/llvm/MC/MCAsmInfoDarwin.h Mon Dec 29 09:18:31 2014
>> @@ -19,9 +19,9 @@
>>
>>  namespace llvm {
>>    class MCAsmInfoDarwin : public MCAsmInfo {
>> -    virtual void anchor();
>>    public:
>>      explicit MCAsmInfoDarwin();
>> +    bool isSectionAtomizableBySymbols(const MCSection &Section) const
>> override;
>>    };
>>  }
>>
>>
>> Modified: llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h (original)
>> +++ llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h Mon Dec 29
>> 09:18:31 2014
>> @@ -151,12 +151,6 @@ public:
>>      return nullptr;
>>    }
>>
>> -  /// \brief True if the section is atomized using the symbols in it.
>> -  /// This is false if the section is not atomized at all (most ELF
>> sections) or
>> -  /// if it is atomized based on its contents (MachO' __TEXT,__cstring
>> for
>> -  /// example).
>> -  virtual bool isSectionAtomizableBySymbols(const MCSection &Section)
>> const;
>> -
>>  protected:
>>    virtual const MCSection *
>>    SelectSectionForGlobal(const GlobalValue *GV, SectionKind Kind,
>>
>> Modified: llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp Mon Dec 29
>> 09:18:31 2014
>> @@ -573,60 +573,6 @@ const MCSection *TargetLoweringObjectFil
>>    return S;
>>  }
>>
>> -bool TargetLoweringObjectFileMachO::isSectionAtomizableBySymbols(
>> -    const MCSection &Section) const {
>> -    const MCSectionMachO &SMO = static_cast<const
>> MCSectionMachO&>(Section);
>> -
>> -    // Sections holding 1 byte strings are atomized based on the data
>> -    // they contain.
>> -    // Sections holding 2 byte strings require symbols in order to be
>> -    // atomized.
>> -    // There is no dedicated section for 4 byte strings.
>> -    if (SMO.getKind().isMergeable1ByteCString())
>> -      return false;
>> -
>> -    if (SMO.getSegmentName() == "__TEXT" &&
>> -        SMO.getSectionName() == "__objc_classname" &&
>> -        SMO.getType() == MachO::S_CSTRING_LITERALS)
>> -      return false;
>> -
>> -    if (SMO.getSegmentName() == "__TEXT" &&
>> -        SMO.getSectionName() == "__objc_methname" &&
>> -        SMO.getType() == MachO::S_CSTRING_LITERALS)
>> -      return false;
>> -
>> -    if (SMO.getSegmentName() == "__TEXT" &&
>> -        SMO.getSectionName() == "__objc_methtype" &&
>> -        SMO.getType() == MachO::S_CSTRING_LITERALS)
>> -      return false;
>> -
>> -    if (SMO.getSegmentName() == "__DATA" &&
>> -        SMO.getSectionName() == "__cfstring")
>> -      return false;
>> -
>> -    // no_dead_strip sections are not atomized in practice.
>> -    if (SMO.hasAttribute(MachO::S_ATTR_NO_DEAD_STRIP))
>> -      return false;
>> -
>> -    switch (SMO.getType()) {
>> -    default:
>> -      return true;
>> -
>> -      // These sections are atomized at the element boundaries without
>> using
>> -      // symbols.
>> -    case MachO::S_4BYTE_LITERALS:
>> -    case MachO::S_8BYTE_LITERALS:
>> -    case MachO::S_16BYTE_LITERALS:
>> -    case MachO::S_LITERAL_POINTERS:
>> -    case MachO::S_NON_LAZY_SYMBOL_POINTERS:
>> -    case MachO::S_LAZY_SYMBOL_POINTERS:
>> -    case MachO::S_MOD_INIT_FUNC_POINTERS:
>> -    case MachO::S_MOD_TERM_FUNC_POINTERS:
>> -    case MachO::S_INTERPOSING:
>> -      return false;
>> -    }
>> -}
>> -
>>  const MCSection *TargetLoweringObjectFileMachO::
>>  SelectSectionForGlobal(const GlobalValue *GV, SectionKind Kind,
>>                         Mangler &Mang, const TargetMachine &TM) const {
>>
>> Modified: llvm/trunk/lib/MC/MCAsmInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfo.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCAsmInfo.cpp (original)
>> +++ llvm/trunk/lib/MC/MCAsmInfo.cpp Mon Dec 29 09:18:31 2014
>> @@ -109,6 +109,10 @@ MCAsmInfo::MCAsmInfo() {
>>  MCAsmInfo::~MCAsmInfo() {
>>  }
>>
>> +bool MCAsmInfo::isSectionAtomizableBySymbols(const MCSection &Section)
>> const {
>> +  return false;
>> +}
>> +
>>  const MCExpr *
>>  MCAsmInfo::getExprForPersonalitySymbol(const MCSymbol *Sym,
>>                                         unsigned Encoding,
>>
>> Modified: llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp (original)
>> +++ llvm/trunk/lib/MC/MCAsmInfoDarwin.cpp Mon Dec 29 09:18:31 2014
>> @@ -15,10 +15,61 @@
>>  #include "llvm/MC/MCAsmInfoDarwin.h"
>>  #include "llvm/MC/MCContext.h"
>>  #include "llvm/MC/MCExpr.h"
>> +#include "llvm/MC/MCSectionMachO.h"
>>  #include "llvm/MC/MCStreamer.h"
>>  using namespace llvm;
>>
>> -void MCAsmInfoDarwin::anchor() { }
>> +bool MCAsmInfoDarwin::isSectionAtomizableBySymbols(
>> +    const MCSection &Section) const {
>> +  const MCSectionMachO &SMO = static_cast<const MCSectionMachO
>> &>(Section);
>> +
>> +  // Sections holding 1 byte strings are atomized based on the data they
>> +  // contain.
>> +  // Sections holding 2 byte strings require symbols in order to be
>> atomized.
>> +  // There is no dedicated section for 4 byte strings.
>> +  if (SMO.getKind().isMergeable1ByteCString())
>> +    return false;
>> +
>> +  if (SMO.getSegmentName() == "__TEXT" &&
>> +      SMO.getSectionName() == "__objc_classname" &&
>> +      SMO.getType() == MachO::S_CSTRING_LITERALS)
>> +    return false;
>> +
>> +  if (SMO.getSegmentName() == "__TEXT" &&
>> +      SMO.getSectionName() == "__objc_methname" &&
>> +      SMO.getType() == MachO::S_CSTRING_LITERALS)
>> +    return false;
>> +
>> +  if (SMO.getSegmentName() == "__TEXT" &&
>> +      SMO.getSectionName() == "__objc_methtype" &&
>> +      SMO.getType() == MachO::S_CSTRING_LITERALS)
>> +    return false;
>> +
>> +  if (SMO.getSegmentName() == "__DATA" && SMO.getSectionName() ==
>> "__cfstring")
>> +    return false;
>> +
>> +  // no_dead_strip sections are not atomized in practice.
>> +  if (SMO.hasAttribute(MachO::S_ATTR_NO_DEAD_STRIP))
>> +    return false;
>> +
>> +  switch (SMO.getType()) {
>> +  default:
>> +    return true;
>> +
>> +  // These sections are atomized at the element boundaries without using
>> +  // symbols.
>> +  case MachO::S_4BYTE_LITERALS:
>> +  case MachO::S_8BYTE_LITERALS:
>> +  case MachO::S_16BYTE_LITERALS:
>> +  case MachO::S_LITERAL_POINTERS:
>> +  case MachO::S_NON_LAZY_SYMBOL_POINTERS:
>> +  case MachO::S_LAZY_SYMBOL_POINTERS:
>> +  case MachO::S_MOD_INIT_FUNC_POINTERS:
>> +  case MachO::S_MOD_TERM_FUNC_POINTERS:
>> +  case MachO::S_INTERPOSING:
>> +    return false;
>> +  }
>> +}
>>
>>  MCAsmInfoDarwin::MCAsmInfoDarwin() {
>>    // Common settings for all Darwin targets.
>>
>> Modified: llvm/trunk/lib/MC/MCAssembler.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCAssembler.cpp (original)
>> +++ llvm/trunk/lib/MC/MCAssembler.cpp Mon Dec 29 09:18:31 2014
>> @@ -12,6 +12,7 @@
>>  #include "llvm/ADT/StringExtras.h"
>>  #include "llvm/ADT/Twine.h"
>>  #include "llvm/MC/MCAsmBackend.h"
>> +#include "llvm/MC/MCAsmInfo.h"
>>  #include "llvm/MC/MCAsmLayout.h"
>>  #include "llvm/MC/MCCodeEmitter.h"
>>  #include "llvm/MC/MCContext.h"
>> @@ -448,8 +449,8 @@ const MCSymbolData *MCAssembler::getAtom
>>
>>    // Non-linker visible symbols in sections which can't be atomized have
>> no
>>    // defining atom.
>> -  if (!getBackend().isSectionAtomizable(
>> -        SD->getFragment()->getParent()->getSection()))
>> +  if (!getContext().getAsmInfo()->isSectionAtomizableBySymbols(
>> +          SD->getFragment()->getParent()->getSection()))
>>      return nullptr;
>>
>>    // Otherwise, return the atom for the containing fragment.
>>
>> Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.h (original)
>> +++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCAsmInfo.h Mon Dec 29
>> 09:18:31 2014
>> @@ -21,7 +21,8 @@
>>  namespace llvm {
>>
>>    class ARMMCAsmInfoDarwin : public MCAsmInfoDarwin {
>> -    void anchor() override;
>> +    virtual void anchor();
>> +
>>    public:
>>      explicit ARMMCAsmInfoDarwin(StringRef TT);
>>    };
>>
>> Modified: llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h (original)
>> +++ llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h Mon Dec 29
>> 09:18:31 2014
>> @@ -21,7 +21,8 @@ namespace llvm {
>>  class Triple;
>>
>>    class PPCMCAsmInfoDarwin : public MCAsmInfoDarwin {
>> -    void anchor() override;
>> +    virtual void anchor();
>> +
>>    public:
>>      explicit PPCMCAsmInfoDarwin(bool is64Bit, const Triple&);
>>    };
>>
>> Modified: llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp (original)
>> +++ llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp Mon Dec 29 09:18:31
>> 2014
>> @@ -270,12 +270,6 @@ SectionForGlobal(const GlobalValue *GV,
>>    return SelectSectionForGlobal(GV, Kind, Mang, TM);
>>  }
>>
>> -bool TargetLoweringObjectFile::isSectionAtomizableBySymbols(
>> -    const MCSection &Section) const {
>> -  return false;
>> -}
>> -
>> -
>>  /// getSectionForConstant - Given a mergable constant with the
>>  /// specified size and relocation information, return a section that it
>>  /// should be placed in.
>>
>> Modified: llvm/trunk/lib/Target/TargetMachine.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/TargetMachine.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/TargetMachine.cpp (original)
>> +++ llvm/trunk/lib/Target/TargetMachine.cpp Mon Dec 29 09:18:31 2014
>> @@ -182,7 +182,8 @@ void TargetMachine::getNameWithPrefix(Sm
>>    const TargetLoweringObjectFile &TLOF =
>>        getSubtargetImpl()->getTargetLowering()->getObjFileLowering();
>>    const MCSection *TheSection = TLOF.SectionForGlobal(GV, GVKind, Mang,
>> *this);
>> -  bool CannotUsePrivateLabel =
>> TLOF.isSectionAtomizableBySymbols(*TheSection);
>> +  bool CannotUsePrivateLabel =
>> +      AsmInfo->isSectionAtomizableBySymbols(*TheSection);
>>    Mang.getNameWithPrefix(Name, GV, CannotUsePrivateLabel);
>>  }
>>
>>
>> Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp Mon Dec 29
>> 09:18:31 2014
>> @@ -790,26 +790,6 @@ public:
>>      return SMO.getType() == MachO::S_CSTRING_LITERALS;
>>    }
>>
>> -  bool isSectionAtomizable(const MCSection &Section) const override {
>> -    const MCSectionMachO &SMO = static_cast<const
>> MCSectionMachO&>(Section);
>> -    // Fixed sized data sections are uniqued, they cannot be diced into
>> atoms.
>> -    switch (SMO.getType()) {
>> -    default:
>> -      return true;
>> -
>> -    case MachO::S_4BYTE_LITERALS:
>> -    case MachO::S_8BYTE_LITERALS:
>> -    case MachO::S_16BYTE_LITERALS:
>> -    case MachO::S_LITERAL_POINTERS:
>> -    case MachO::S_NON_LAZY_SYMBOL_POINTERS:
>> -    case MachO::S_LAZY_SYMBOL_POINTERS:
>> -    case MachO::S_MOD_INIT_FUNC_POINTERS:
>> -    case MachO::S_MOD_TERM_FUNC_POINTERS:
>> -    case MachO::S_INTERPOSING:
>> -      return false;
>> -    }
>> -  }
>> -
>>    /// \brief Generate the compact unwind encoding for the CFI
>> instructions.
>>    uint32_t generateCompactUnwindEncoding(
>>                               ArrayRef<MCCFIInstruction> Instrs) const
>> override {
>>
>> Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h?rev=224935&r1=224934&r2=224935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h (original)
>> +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.h Mon Dec 29
>> 09:18:31 2014
>> @@ -23,7 +23,8 @@ namespace llvm {
>>    class Triple;
>>
>>    class X86MCAsmInfoDarwin : public MCAsmInfoDarwin {
>> -    void anchor() override;
>> +    virtual void anchor();
>> +
>>    public:
>>      explicit X86MCAsmInfoDarwin(const Triple &Triple);
>>    };
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 13889 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150107/aee75dae/attachment.bin>


More information about the llvm-commits mailing list