From 39f35038140d2e21023eefcc7acc65c5b68fc77f Mon Sep 17 00:00:00 2001 From: Sean Fertile Date: Tue, 30 Jul 2019 15:37:01 +0000 Subject: [PATCH] Address post commit review comments on revision 366727. Addresses number of comment made on D64652 after commiting: - Reorders function decls in the TargetLoweringObjectFileXCOFF class. - Fix comment in MCSectionXCOFF to include description of external reference csects. - Convert several llvm_unreachables to report_fatal_error - Convert several dyn_casts to casts as they are expected not to fail. - Avoid copying DataLayout object. llvm-svn: 367324 --- llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h | 12 ++++++------ llvm/include/llvm/MC/MCSectionXCOFF.h | 5 ++++- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp | 10 +++++----- llvm/lib/MC/MCSectionXCOFF.cpp | 2 +- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | 10 +++++----- llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll | 2 -- 6 files changed, 21 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h index 8964d4c..25e19e7 100644 --- a/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h +++ b/llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h @@ -211,16 +211,13 @@ public: TargetLoweringObjectFileXCOFF() = default; ~TargetLoweringObjectFileXCOFF() override = default; - MCSection *getExplicitSectionGlobal(const GlobalObject *GO, SectionKind Kind, - const TargetMachine &TM) const override; - - MCSection *SelectSectionForGlobal(const GlobalObject *GO, SectionKind Kind, - const TargetMachine &TM) const override; + void Initialize(MCContext &Ctx, const TargetMachine &TM) override; bool shouldPutJumpTableInFunctionSection(bool UsesLabelDifference, const Function &F) const override; - void Initialize(MCContext &Ctx, const TargetMachine &TM) override; + MCSection *getExplicitSectionGlobal(const GlobalObject *GO, SectionKind Kind, + const TargetMachine &TM) const override; MCSection *getStaticCtorSection(unsigned Priority, const MCSymbol *KeySym) const override; @@ -230,6 +227,9 @@ public: const MCExpr *lowerRelativeReference(const GlobalValue *LHS, const GlobalValue *RHS, const TargetMachine &TM) const override; + + MCSection *SelectSectionForGlobal(const GlobalObject *GO, SectionKind Kind, + const TargetMachine &TM) const override; }; } // end namespace llvm diff --git a/llvm/include/llvm/MC/MCSectionXCOFF.h b/llvm/include/llvm/MC/MCSectionXCOFF.h index 4b39fd6..fe486f3 100644 --- a/llvm/include/llvm/MC/MCSectionXCOFF.h +++ b/llvm/include/llvm/MC/MCSectionXCOFF.h @@ -28,6 +28,9 @@ class MCSymbol; // will have a label definition representing their offset within the csect. // 2) Uninitialized: The Type will be XTY_CM, it will contain a single symbol, // and may not contain label definitions. +// 3) An external reference providing a symbol table entry for a symbol +// contained in another XCOFF object file. External reference csects are not +// implemented yet. class MCSectionXCOFF final : public MCSection { friend class MCContext; @@ -40,7 +43,7 @@ class MCSectionXCOFF final : public MCSection { : MCSection(SV_XCOFF, K, Begin), Name(Section), MappingClass(SMC), Type(ST) { assert((ST == XCOFF::XTY_SD || ST == XCOFF::XTY_CM) && - "Unexpected type for csect."); + "Invalid or unhandled type for csect."); } public: diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp index a3bf69a..ac7c4c5 100644 --- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp +++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp @@ -1826,7 +1826,7 @@ MCSection *TargetLoweringObjectFileWasm::getStaticDtorSection( //===----------------------------------------------------------------------===// MCSection *TargetLoweringObjectFileXCOFF::getExplicitSectionGlobal( const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const { - llvm_unreachable("XCOFF explicit sections not yet implemented."); + report_fatal_error("XCOFF explicit sections not yet implemented."); } MCSection *TargetLoweringObjectFileXCOFF::SelectSectionForGlobal( @@ -1851,7 +1851,7 @@ MCSection *TargetLoweringObjectFileXCOFF::SelectSectionForGlobal( bool TargetLoweringObjectFileXCOFF::shouldPutJumpTableInFunctionSection( bool UsesLabelDifference, const Function &F) const { - llvm_unreachable("TLOF XCOFF not yet implemented."); + report_fatal_error("TLOF XCOFF not yet implemented."); } void TargetLoweringObjectFileXCOFF::Initialize(MCContext &Ctx, @@ -1864,16 +1864,16 @@ void TargetLoweringObjectFileXCOFF::Initialize(MCContext &Ctx, MCSection *TargetLoweringObjectFileXCOFF::getStaticCtorSection( unsigned Priority, const MCSymbol *KeySym) const { - llvm_unreachable("XCOFF ctor section not yet implemented."); + report_fatal_error("XCOFF ctor section not yet implemented."); } MCSection *TargetLoweringObjectFileXCOFF::getStaticDtorSection( unsigned Priority, const MCSymbol *KeySym) const { - llvm_unreachable("XCOFF dtor section not yet implemented."); + report_fatal_error("XCOFF dtor section not yet implemented."); } const MCExpr *TargetLoweringObjectFileXCOFF::lowerRelativeReference( const GlobalValue *LHS, const GlobalValue *RHS, const TargetMachine &TM) const { - llvm_unreachable("XCOFF not yet implemented."); + report_fatal_error("XCOFF not yet implemented."); } diff --git a/llvm/lib/MC/MCSectionXCOFF.cpp b/llvm/lib/MC/MCSectionXCOFF.cpp index 740365f..9cee1cc 100644 --- a/llvm/lib/MC/MCSectionXCOFF.cpp +++ b/llvm/lib/MC/MCSectionXCOFF.cpp @@ -32,7 +32,7 @@ void MCSectionXCOFF::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T, if (getMappingClass() != XCOFF::XMC_RW) llvm_unreachable("Unsupported storage-mapping class for common csect"); if (getCSectType() != XCOFF::XTY_CM) - llvm_unreachable("wrong csect type for common csect"); + llvm_unreachable("wrong csect type for .bss csect"); // Don't have to print a directive for switching to section for commons. // '.comm' and '.lcomm' directives for the variable will create the needed // csect. diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp index 237c8da..890a0dd 100644 --- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp +++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp @@ -1656,20 +1656,20 @@ void PPCAIXAsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) { report_fatal_error("Custom section for Data not yet supported."); if (GV->hasComdat()) - report_fatal_error("COMDAT not yet supported on AIX."); + report_fatal_error("COMDAT not yet supported by AIX."); SectionKind GVKind = getObjFileLowering().getKindForGlobal(GV, TM); if (!GVKind.isCommon()) - report_fatal_error("Only common variables are supported on AIX."); + report_fatal_error("Only common variables are supported on AIX for now."); // Create the containing csect and switch to it. - MCSectionXCOFF *CSect = dyn_cast( + MCSectionXCOFF *CSect = cast( getObjFileLowering().SectionForGlobal(GV, GVKind, TM)); OutStreamer->SwitchSection(CSect); // Create the symbol and emit it. - MCSymbolXCOFF *XSym = dyn_cast(getSymbol(GV)); - auto DL = GV->getParent()->getDataLayout(); + MCSymbolXCOFF *XSym = cast(getSymbol(GV)); + const DataLayout &DL = GV->getParent()->getDataLayout(); unsigned Align = GV->getAlignment() ? GV->getAlignment() : DL.getPreferredAlignment(GV); uint64_t Size = DL.getTypeAllocSize(GV->getType()->getElementType()); diff --git a/llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll b/llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll index 0774e9f..3aea223 100644 --- a/llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll +++ b/llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll @@ -6,7 +6,6 @@ @d = common local_unnamed_addr global double 0.000000e+00, align 8 @f = common local_unnamed_addr global float 0.000000e+00, align 4 -@comm = common local_unnamed_addr global double 0.000000e+00, align 8 @over_aligned = common local_unnamed_addr global double 0.000000e+00, align 32 @@ -19,6 +18,5 @@ ; CHECK-NEXT: .comm c,2,1 ; CHECK-NEXT: .comm d,8,3 ; CHECK-NEXT: .comm f,4,2 -; CHECK-NEXT: .comm comm,8,3 ; CHECK-NEXT: .comm over_aligned,8,5 ; CHECK-NEXT: .comm array,32,0 -- 2.7.4