From: Ayke van Laethem Date: Mon, 2 Jan 2023 00:21:19 +0000 (+0100) Subject: [AVR] correctly declare __do_copy_data and __do_clear_bss X-Git-Tag: upstream/17.0.6~21782 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=167338de9687a8e65672001aa4c4f2e62cf45a76;p=platform%2Fupstream%2Fllvm.git [AVR] correctly declare __do_copy_data and __do_clear_bss These two symbols are declared in object files to indicate whether .data needs to be copied from flash or .bss needs to be cleared. They are supported on avr-gcc and reduce firmware size a bit, which is especially important on very small chips. I checked the behavior of avr-gcc and matched it as well as possible. From my investigation, it seems to work as follows: __do_copy_data is set when the compiler finds a data symbol: * without a section name * with a section name starting with ".data" or ".gnu.linkonce.d" * with a section name starting with ".rodata" or ".gnu.linkonce.r" and flash and RAM are in the same address space __do_clear_bss is set when the compiler finds a data symbol: * without a section name * with a section name that starts with .bss Simply checking whether the calculated section name starts with ".data", ".rodata" or ".bss" should result in the same behavior. Fixes: https://github.com/llvm/llvm-project/issues/58857 Differential Revision: https://reviews.llvm.org/D140830 --- diff --git a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp index 9128674..897a45e 100644 --- a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp +++ b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp @@ -19,6 +19,7 @@ #include "MCTargetDesc/AVRMCExpr.h" #include "TargetInfo/AVRTargetInfo.h" +#include "llvm/BinaryFormat/ELF.h" #include "llvm/CodeGen/AsmPrinter.h" #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineInstr.h" @@ -28,11 +29,13 @@ #include "llvm/IR/Mangler.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCInst.h" +#include "llvm/MC/MCSectionELF.h" #include "llvm/MC/MCStreamer.h" #include "llvm/MC/MCSymbol.h" #include "llvm/MC/TargetRegistry.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Target/TargetLoweringObjectFile.h" #define DEBUG_TYPE "avr-asm-printer" @@ -229,21 +232,51 @@ void AVRAsmPrinter::emitXXStructor(const DataLayout &DL, const Constant *CV) { } bool AVRAsmPrinter::doFinalization(Module &M) { + const TargetLoweringObjectFile &TLOF = getObjFileLowering(); + const AVRTargetMachine &TM = (const AVRTargetMachine &)MMI->getTarget(); + const AVRSubtarget *SubTM = (const AVRSubtarget *)TM.getSubtargetImpl(); + + bool NeedsCopyData = false; + bool NeedsClearBSS = false; + for (const auto &GO : M.globals()) { + if (!GO.hasInitializer() || GO.hasAvailableExternallyLinkage()) + // These globals aren't defined in the current object file. + continue; + + if (GO.hasCommonLinkage()) { + // COMMON symbols are put in .bss. + NeedsClearBSS = true; + continue; + } + + auto *Section = cast(TLOF.SectionForGlobal(&GO, TM)); + if (Section->getName().startswith(".data")) + NeedsCopyData = true; + else if (Section->getName().startswith(".rodata") && SubTM->hasPROGMEM()) + // AVRs that have a separate PROGMEM (that's most AVRs) store .rodata + // sections in RAM. + NeedsCopyData = true; + else if (Section->getName().startswith(".bss")) + NeedsClearBSS = true; + } + MCSymbol *DoCopyData = OutContext.getOrCreateSymbol("__do_copy_data"); MCSymbol *DoClearBss = OutContext.getOrCreateSymbol("__do_clear_bss"); - // FIXME: We can disable __do_copy_data if there are no static RAM variables. - - OutStreamer->emitRawComment( - " Declaring this symbol tells the CRT that it should"); - OutStreamer->emitRawComment( - "copy all variables from program memory to RAM on startup"); - OutStreamer->emitSymbolAttribute(DoCopyData, MCSA_Global); + if (NeedsCopyData) { + OutStreamer->emitRawComment( + " Declaring this symbol tells the CRT that it should"); + OutStreamer->emitRawComment( + "copy all variables from program memory to RAM on startup"); + OutStreamer->emitSymbolAttribute(DoCopyData, MCSA_Global); + } - OutStreamer->emitRawComment( - " Declaring this symbol tells the CRT that it should"); - OutStreamer->emitRawComment("clear the zeroed data section on startup"); - OutStreamer->emitSymbolAttribute(DoClearBss, MCSA_Global); + if (NeedsClearBSS) { + OutStreamer->emitRawComment( + " Declaring this symbol tells the CRT that it should"); + OutStreamer->emitRawComment("clear the zeroed data section on startup"); + OutStreamer->emitSymbolAttribute(DoClearBss, MCSA_Global); + } return AsmPrinter::doFinalization(M); } diff --git a/llvm/lib/Target/AVR/AVRDevices.td b/llvm/lib/Target/AVR/AVRDevices.td index 8ecb0a1..f2c8a2e 100644 --- a/llvm/lib/Target/AVR/AVRDevices.td +++ b/llvm/lib/Target/AVR/AVRDevices.td @@ -65,6 +65,11 @@ def FeatureMOVW : SubtargetFeature<"movw", "m_hasMOVW", "true", "The device supports the 16-bit MOVW " "instruction">; +// The device has a separate flash namespace that must be accessed using special +// instructions like lpm. +def FeaturePROGMEM : SubtargetFeature<"progmem", "m_hasPROGMEM", "true", + "The device has a separate flash namespace">; + // The device supports the `LPM` instruction, with implied destination being r0. def FeatureLPM : SubtargetFeature<"lpm", "m_hasLPM", "true", "The device supports the `LPM` instruction">; @@ -156,7 +161,7 @@ def ELFArchXMEGA7 : ELFArch<"EF_AVR_ARCH_XMEGA7">; // device should have. def FamilyAVR0 : Family<"avr0", []>; -def FamilyAVR1 : Family<"avr1", [FamilyAVR0, FeatureLPM, FeatureMMR]>; +def FamilyAVR1 : Family<"avr1", [FamilyAVR0, FeatureLPM, FeaturePROGMEM, FeatureMMR]>; def FamilyAVR2 : Family<"avr2", @@ -192,13 +197,13 @@ def FamilyTiny FeatureSmallStack]>; def FamilyXMEGA3 : Family<"xmega3", - [FamilyAVR0, FeatureLPM, FeatureIJMPCALL, + [FamilyAVR0, FeatureLPM, FeaturePROGMEM, FeatureIJMPCALL, FeatureADDSUBIW, FeatureSRAM, FeatureJMPCALL, FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureBREAK]>; def FamilyXMEGA : Family<"xmega", - [FamilyAVR0, FeatureLPM, FeatureIJMPCALL, + [FamilyAVR0, FeatureLPM, FeaturePROGMEM, FeatureIJMPCALL, FeatureADDSUBIW, FeatureSRAM, FeatureJMPCALL, FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM, FeatureBREAK, FeatureEIJMPCALL, diff --git a/llvm/lib/Target/AVR/AVRSubtarget.h b/llvm/lib/Target/AVR/AVRSubtarget.h index ee16ed4..10ae4d2 100644 --- a/llvm/lib/Target/AVR/AVRSubtarget.h +++ b/llvm/lib/Target/AVR/AVRSubtarget.h @@ -73,6 +73,7 @@ public: bool hasLPMX() const { return m_hasLPMX; } bool hasELPM() const { return m_hasELPM; } bool hasELPMX() const { return m_hasELPMX; } + bool hasPROGMEM() const { return m_hasPROGMEM; } bool hasSPM() const { return m_hasSPM; } bool hasSPMX() const { return m_hasSPMX; } bool hasDES() const { return m_hasDES; } @@ -127,6 +128,7 @@ private: bool m_hasLPMX; bool m_hasELPM; bool m_hasELPMX; + bool m_hasPROGMEM; bool m_hasSPM; bool m_hasSPMX; bool m_hasDES; diff --git a/llvm/test/CodeGen/AVR/clear-bss.ll b/llvm/test/CodeGen/AVR/clear-bss.ll index 9c63815..a822610 100644 --- a/llvm/test/CodeGen/AVR/clear-bss.ll +++ b/llvm/test/CodeGen/AVR/clear-bss.ll @@ -1,5 +1,5 @@ ; RUN: llc < %s -march=avr | FileCheck %s ; CHECK: .globl __do_clear_bss -@zeroed = internal constant [3 x i8] zeroinitializer - +@zeroed = internal global [3 x i8] zeroinitializer +@common = common global i8 0 diff --git a/llvm/test/CodeGen/AVR/no-clear-bss.ll b/llvm/test/CodeGen/AVR/no-clear-bss.ll new file mode 100644 index 0000000..4a25747 --- /dev/null +++ b/llvm/test/CodeGen/AVR/no-clear-bss.ll @@ -0,0 +1,8 @@ +; RUN: llc < %s -march=avr | FileCheck %s + +; CHECK: .globl __do_copy_data +; CHECK-NOT: .globl __do_clear_bss + +@str = internal global [3 x i8] c"foo" +@noinit = internal constant [3 x i8] zeroinitializer, section ".noinit" +@external = external constant [3 x i8] diff --git a/llvm/test/CodeGen/AVR/no-copy-data.ll b/llvm/test/CodeGen/AVR/no-copy-data.ll new file mode 100644 index 0000000..4875296 --- /dev/null +++ b/llvm/test/CodeGen/AVR/no-copy-data.ll @@ -0,0 +1,8 @@ +; RUN: llc < %s -march=avr | FileCheck %s + +; CHECK-NOT: .globl __do_copy_data +; CHECK: .globl __do_clear_bss + +@noinit = internal global i8 5, section ".noinit" +@external = external global i8 +@global = global i8 0