[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 (file)
--- 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<MCSectionELF>(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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
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 (file)
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