[AVR] correctly declare __do_copy_data and __do_clear_bss

[aykevl]

Summary

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.

[aykevl]

llvm/test/CodeGen/AVR/clear-bss.ll

4–5

This was an incorrect test: constant globals are actually stored in .rodata which is allocated in RAM. Making it a global makes it a .bss value.

[benshi001]

You can add the lines I suggested while committing.

llvm/test/CodeGen/AVR/no-clear-bss.ll

5

add a new line CHECK: .globl __do_copy_data

llvm/test/CodeGen/AVR/no-copy-data.ll

3

add CHECK: .globl __do_clear_bss

[benshi001]

Please also add Fixes https://github.com/llvm/llvm-project/issues/58857 in your commit message.

[aykevl]

• add CHECK-NOT lines to tests
• add zeroed global to the no-copy-data.ll test

[aykevl]

llvm/test/CodeGen/AVR/no-copy-data.ll

3

__do_clear_bss isn't set either: both globals aren't in .bss (one is in a different section, the other is not defined but declared).
I have added a new global to be able to test this. Do you think this is a good solution?

[benshi001]

llvm/test/CodeGen/AVR/no-copy-data.ll

3

That is fine. Thanks!

[vitalybuka]

The patch introduces use of uninitialized value:
https://lab.llvm.org/buildbot/#/builders/74/builds/16422/steps/18/logs/stdio

[aykevl]

@vitalybuka oops, thank you for the fix!

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

 #include "AVR.h"
 #include "AVRMCInstLower.h"
 #include "AVRSubtarget.h"
 #include "AVRTargetMachine.h"
 #include "MCTargetDesc/AVRInstPrinter.h"
 #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"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #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"
 
 namespace llvm {
 
 /// An AVR assembly code printer.
 class AVRAsmPrinter : public AsmPrinter {
 public:
if (!EmittedStructorSymbolAttrs) {
 
     EmittedStructorSymbolAttrs = true;
   }
 
   AsmPrinter::emitXXStructor(DL, 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.
-
+  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);
+  }
 
+  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);
 }
 
 void AVRAsmPrinter::emitStartOfAsmFile(Module &M) {
   const AVRTargetMachine &TM = (const AVRTargetMachine &)MMI->getTarget();
   const AVRSubtarget *SubTM = (const AVRSubtarget *)TM.getSubtargetImpl();
   if (!SubTM)

llvm/lib/Target/AVR/AVRDevices.td

: SubtargetFeature<"smallstack", "m_hasSmallStack", "true",
                        "The device has an 8-bit "
                        "stack pointer">;
 
 // The device supports the 16-bit GPR pair MOVW instruction.
 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">;
 
 // The device supports the `LPM Rd, Z[+] instruction.
 def FeatureLPMX : SubtargetFeature<"lpmx", "m_hasLPMX", "true",
                                    "The device supports the `LPM Rd, Z[+]` "
                                    "instruction">;

 //===---------------------------------------------------------------------===//
 // AVR Families
 //===---------------------------------------------------------------------===//
 
 // The device has at least the bare minimum that **every** single AVR
 // 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",
              [FamilyAVR1, FeatureIJMPCALL, FeatureADDSUBIW, FeatureSRAM]>;
 
 def FamilyAVR25
     : Family<"avr25",
              [FamilyAVR2, FeatureMOVW, FeatureLPMX, FeatureSPM, FeatureBREAK]>;

 def FamilyAVR6 : Family<"avr6", [FamilyAVR51, FeatureEIJMPCALL]>;
 
 def FamilyTiny
     : Family<"avrtiny",
              [FamilyAVR0, FeatureBREAK, FeatureSRAM, FeatureTinyEncoding,
               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,
                           FeatureSPMX, FeatureDES, FeatureELPM, FeatureELPMX]>;
 
 def FamilyXMEGAU : Family<"xmegau", [FamilyXMEGA, FeatureRMW]>;
 
 def FeatureSetSpecial

llvm/lib/Target/AVR/AVRSubtarget.h

public:
   bool hasEIJMPCALL() const { return m_hasEIJMPCALL; }
   bool hasADDSUBIW() const { return m_hasADDSUBIW; }
   bool hasSmallStack() const { return m_hasSmallStack; }
   bool hasMOVW() const { return m_hasMOVW; }
   bool hasLPM() const { return m_hasLPM; }
   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; }
   bool supportsRMW() const { return m_supportsRMW; }
   bool supportsMultiplication() const { return m_supportsMultiplication; }
   bool hasBREAK() const { return m_hasBREAK; }
   bool hasTinyEncoding() const { return m_hasTinyEncoding; }
   bool hasMemMappedGPR() const { return m_hasMemMappedGPR; }
private:
   bool m_hasEIJMPCALL;
   bool m_hasADDSUBIW;
   bool m_hasSmallStack;
   bool m_hasMOVW;
   bool m_hasLPM;
   bool m_hasLPMX;
   bool m_hasELPM;
   bool m_hasELPMX;
+  bool m_hasPROGMEM;
   bool m_hasSPM;
   bool m_hasSPMX;
   bool m_hasDES;
   bool m_supportsRMW;
   bool m_supportsMultiplication;
   bool m_hasBREAK;
   bool m_hasTinyEncoding;
   bool m_hasMemMappedGPR;

llvm/test/CodeGen/AVR/clear-bss.ll

 ; 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

llvm/test/CodeGen/AVR/no-clear-bss.ll

+; 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]

llvm/test/CodeGen/AVR/no-copy-data.ll

+; 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