[AVR] Handle flash RO data mapped to data space for newer devices #146244

tomtor · 2025-06-28T21:42:34Z

Newer AVR devices map (part of the) flash to a 32kB window at 0x8000 in the data/IO space.

The linker correctly loads readonly data at 0x8000, but we currently always pull in __do_copy_data when we encounter RO data.
This is not needed when we have no initialized data.

Newer versions of avr-ld (invoked via avr-gcc) handle this (calling __do_copy_data when needed) correctly, and users of newer devices must always install recent avr-ld and avr-lib versions to gain access to loader spec files and crt files. For older devices and older avr-ld the old approach remains unchanged.

Also add some support for devices with an flmap register (xmega2 and xmega4 with >= 64kB flash)., by adding gcc compatible options.

llvmbot · 2025-06-28T21:43:02Z

@llvm/pr-subscribers-clang

Author: Tom Vijlbrief (tomtor)

Changes

Newer AVR devices map (part of the) flash to a 32kB window at 0x8000 in the data/IO space.

The linker correctly loads readonly data at 0x8000, but we currently always pull in __do_copy_data when we encounter RO data.
This fails when we have no other initialized data in .data because the copy loop expects to copy at least 1 byte. When the size is 0 it tries to copy 0x10000 bytes.

Newer versions of avr-ld (invoked via avr-gcc) handle this correctly, and users of newer devices must always install recent avr-ld and avr-lib versions to gain access to loader spec files and crt files. For older devices and older avr-ld the old approach remains unchanged.

Also add some support for devices with an flmap register (xmega2 and xmega4 with >= 64kB flash).

Full diff: https://github.com/llvm/llvm-project/pull/146244.diff

5 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+4)
(modified) clang/lib/Basic/Targets/AVR.cpp (+6)
(modified) clang/lib/Driver/ToolChains/AVR.cpp (+13-2)
(modified) llvm/lib/Target/AVR/AVRAsmPrinter.cpp (+10-4)
(modified) llvm/lib/Target/AVR/AVRDevices.td (+34-22)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0ffd8c40da7da..3a6f1bb2669a1 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4886,6 +4886,10 @@ def mguard_EQ : Joined<["-"], "mguard=">, Group<m_Group>,
   HelpText<"Enable or disable Control Flow Guard checks and guard tables emission">,
   Values<"none,cf,cf-nochecks">;
 def mmcu_EQ : Joined<["-"], "mmcu=">, Group<m_Group>;
+def mflmap : Flag<["-"], "mflmap">, Group<m_Group>,
+  HelpText<"Use AVR mapped memory for RO data">;
+def mrodata_in_ram : Flag<["-"], "mrodata-in-ram">, Group<m_Group>,
+  HelpText<"Copy RO data to ram">;
 def msim : Flag<["-"], "msim">, Group<m_Group>;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group<clang_ignored_m_Group>;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group<clang_ignored_m_Group>;
diff --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index bbe7b01ca036d..448e72206776f 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -418,6 +418,10 @@ static MCUInfo AVRMcus[] = {
 } // namespace targets
 } // namespace clang
 
+static bool ArchHasFLMAP(StringRef Name) {
+  return Name.starts_with("avr64") or Name.starts_with("avr128");
+}
+
 static bool ArchHasELPM(StringRef Arch) {
   return llvm::StringSwitch<bool>(Arch)
     .Cases("31", "51", "6", true)
@@ -529,6 +533,8 @@ void AVRTargetInfo::getTargetDefines(const LangOptions &Opts,
   Builder.defineMacro("__AVR_ARCH__", Arch);
 
   // TODO: perhaps we should use the information from AVRDevices.td instead?
+  if (ArchHasFLMAP(DefineName))
+    Builder.defineMacro("__AVR_HAVE_FLMAP__");
   if (ArchHasELPM(Arch))
     Builder.defineMacro("__AVR_HAVE_ELPM__");
   if (ArchHasELPMX(Arch))
diff --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index 731076d9754a9..155e0d812c95f 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -651,8 +651,19 @@ void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   // This is almost always required because otherwise avr-ld
   // will assume 'avr2' and warn about the program being larger
   // than the bare minimum supports.
-  if (Linker.find("avr-ld") != std::string::npos && FamilyName)
-    CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+  if (Linker.find("avr-ld") != std::string::npos && FamilyName) {
+    // Option to use mapped memory for modern devices with >32kB flash.
+    // This is the only option for modern devices with <= 32kB flash,
+    // but the larger default to a copy from flash to RAM (avr-ld version < 14)
+    // or map the highest 32kB to RAM (avr-ld version >= 14).
+    if (Args.hasFlag(options::OPT_mflmap, options::OPT_mrodata_in_ram, false)) {
+      CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName + "_flmap"));
+      CmdArgs.push_back(Args.MakeArgString(std::string("-u")));
+      CmdArgs.push_back(Args.MakeArgString(std::string("__do_flmap_init")));
+    }
+    else
+      CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+  }
 
   C.addCommand(std::make_unique<Command>(
       JA, *this, ResponseFileSupport::AtFileCurCP(), Args.MakeArgString(Linker),
diff --git a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
index ad8aa5717fb42..decfcc4b67f5d 100644
--- a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
+++ b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
@@ -263,11 +263,17 @@ bool AVRAsmPrinter::doFinalization(Module &M) {
     auto *Section = cast<MCSectionELF>(TLOF.SectionForGlobal(&GO, TM));
     if (Section->getName().starts_with(".data"))
       NeedsCopyData = true;
-    else if (Section->getName().starts_with(".rodata") && SubTM->hasLPM())
+    else if (Section->getName().starts_with(".rodata") && SubTM->hasLPM()) {
       // AVRs that have a separate program memory (that's most AVRs) store
-      // .rodata sections in RAM.
-      NeedsCopyData = true;
-    else if (Section->getName().starts_with(".bss"))
+      // .rodata sections in RAM,
+      // but XMEGA3 family maps all flash in the data space.
+      // Invoking __do_copy_data with 0 bytes to copy will crash,
+      // so we let the loader handle this for newer devices.
+      if (!(SubTM->hasFeatureSetFamilyXMEGA2() ||
+            SubTM->hasFeatureSetFamilyXMEGA3() ||
+            SubTM->hasFeatureSetFamilyXMEGA4()))
+        NeedsCopyData = true;
+    } else if (Section->getName().starts_with(".bss"))
       NeedsClearBSS = true;
   }
 
diff --git a/llvm/lib/Target/AVR/AVRDevices.td b/llvm/lib/Target/AVR/AVRDevices.td
index ad760d7403573..0b6b0e3696e5f 100644
--- a/llvm/lib/Target/AVR/AVRDevices.td
+++ b/llvm/lib/Target/AVR/AVRDevices.td
@@ -49,6 +49,16 @@ def FeatureEIJMPCALL : SubtargetFeature<"eijmpcall", "HasEIJMPCALL", "true",
                                         "The device supports the "
                                         "`EIJMP`/`EICALL` instructions">;
 
+// The device supports `FLMAP` flash bank data mapping to the data space.
+def FeatureFLMAP : SubtargetFeature<"flmap", "HasFLMAP", "true",
+                                        "The device supports the "
+                                        "`FLMAP` mapping">;
+
+// The device maps all flash (<= 32kB) to the data space.
+def FeatureMAP : SubtargetFeature<"map", "HasMAP", "true",
+                                        "The device maps all flash (<= 32kB) "
+                                        "to the data space">;
+
 // The device supports `ADDI Rd, K`, `SUBI Rd, K`.
 def FeatureADDSUBIW : SubtargetFeature<"addsubiw", "HasADDSUBIW", "true",
                                        "Enable 16-bit register-immediate "
@@ -220,6 +230,7 @@ def FamilyXMEGA3 : Family<"xmega3",
                           [FamilyAVR0, FeatureLPM, FeatureIJMPCALL,
                            FeatureADDSUBIW, FeatureSRAM, FeatureJMPCALL,
                            FeatureMultiplication, FeatureMOVW, FeatureLPMX,
+                           FeatureMAP,
                            FeatureBREAK, FeatureLowByteFirst]>;
 
 def FamilyXMEGA4 : Family<"xmega4",
@@ -228,6 +239,7 @@ def FamilyXMEGA4 : Family<"xmega4",
                            FeatureMultiplication, FeatureMOVW, FeatureLPMX,
                            FeatureELPM, FeatureELPMX,
                            FeatureSPM, FeatureSPMX,
+                           FeatureFLMAP,
                            FeatureBREAK, FeatureLowByteFirst]>;
 
 def FamilyXMEGA : Family<"xmega",
@@ -275,9 +287,9 @@ def : Device<"avr5", FamilyAVR5, ELFArchAVR5>;
 def : Device<"avr51", FamilyAVR51, ELFArchAVR51>;
 def : Device<"avr6", FamilyAVR6, ELFArchAVR6>;
 def : Device<"avrxmega1", FamilyXMEGA, ELFArchXMEGA1>;
-def : Device<"avrxmega2", FamilyXMEGA, ELFArchXMEGA2>;
+def : Device<"avrxmega2", FamilyXMEGA2, ELFArchXMEGA2>;
 def : Device<"avrxmega3", FamilyXMEGA3, ELFArchXMEGA3>;
-def : Device<"avrxmega4", FamilyXMEGA, ELFArchXMEGA4>;
+def : Device<"avrxmega4", FamilyXMEGA4, ELFArchXMEGA4>;
 def : Device<"avrxmega5", FamilyXMEGA, ELFArchXMEGA5>;
 def : Device<"avrxmega6", FamilyXMEGA, ELFArchXMEGA6>;
 def : Device<"avrxmega7", FamilyXMEGA, ELFArchXMEGA7>;
@@ -596,26 +608,26 @@ def : Device<"atmega4809", FamilyXMEGA3, ELFArchXMEGA3>;
 
 // Additions from gcc 14:
 
-def : Device<"avr64da28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64da32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64da48", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64da64", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db48", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db64", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd14", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd20", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64du28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64du32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64ea28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64ea32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64ea48", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64sd28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64sd32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64sd48", FamilyXMEGA2, ELFArchXMEGA2>;
+def : Device<"avr64da28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64da32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64da48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64da64", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db64", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd14", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd20", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64du28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64du32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64ea28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64ea32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64ea48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64sd28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64sd32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64sd48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
 
 def : Device<"avr16dd20", FamilyXMEGA3, ELFArchXMEGA3>;
 def : Device<"avr16dd28", FamilyXMEGA3, ELFArchXMEGA3>;

llvmbot · 2025-06-28T21:43:03Z

@llvm/pr-subscribers-clang-driver

Author: Tom Vijlbrief (tomtor)

Changes

Newer AVR devices map (part of the) flash to a 32kB window at 0x8000 in the data/IO space.

The linker correctly loads readonly data at 0x8000, but we currently always pull in __do_copy_data when we encounter RO data.
This fails when we have no other initialized data in .data because the copy loop expects to copy at least 1 byte. When the size is 0 it tries to copy 0x10000 bytes.

Newer versions of avr-ld (invoked via avr-gcc) handle this correctly, and users of newer devices must always install recent avr-ld and avr-lib versions to gain access to loader spec files and crt files. For older devices and older avr-ld the old approach remains unchanged.

Also add some support for devices with an flmap register (xmega2 and xmega4 with >= 64kB flash).

Full diff: https://github.com/llvm/llvm-project/pull/146244.diff

5 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+4)
(modified) clang/lib/Basic/Targets/AVR.cpp (+6)
(modified) clang/lib/Driver/ToolChains/AVR.cpp (+13-2)
(modified) llvm/lib/Target/AVR/AVRAsmPrinter.cpp (+10-4)
(modified) llvm/lib/Target/AVR/AVRDevices.td (+34-22)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0ffd8c40da7da..3a6f1bb2669a1 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4886,6 +4886,10 @@ def mguard_EQ : Joined<["-"], "mguard=">, Group<m_Group>,
   HelpText<"Enable or disable Control Flow Guard checks and guard tables emission">,
   Values<"none,cf,cf-nochecks">;
 def mmcu_EQ : Joined<["-"], "mmcu=">, Group<m_Group>;
+def mflmap : Flag<["-"], "mflmap">, Group<m_Group>,
+  HelpText<"Use AVR mapped memory for RO data">;
+def mrodata_in_ram : Flag<["-"], "mrodata-in-ram">, Group<m_Group>,
+  HelpText<"Copy RO data to ram">;
 def msim : Flag<["-"], "msim">, Group<m_Group>;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group<clang_ignored_m_Group>;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group<clang_ignored_m_Group>;
diff --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index bbe7b01ca036d..448e72206776f 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -418,6 +418,10 @@ static MCUInfo AVRMcus[] = {
 } // namespace targets
 } // namespace clang
 
+static bool ArchHasFLMAP(StringRef Name) {
+  return Name.starts_with("avr64") or Name.starts_with("avr128");
+}
+
 static bool ArchHasELPM(StringRef Arch) {
   return llvm::StringSwitch<bool>(Arch)
     .Cases("31", "51", "6", true)
@@ -529,6 +533,8 @@ void AVRTargetInfo::getTargetDefines(const LangOptions &Opts,
   Builder.defineMacro("__AVR_ARCH__", Arch);
 
   // TODO: perhaps we should use the information from AVRDevices.td instead?
+  if (ArchHasFLMAP(DefineName))
+    Builder.defineMacro("__AVR_HAVE_FLMAP__");
   if (ArchHasELPM(Arch))
     Builder.defineMacro("__AVR_HAVE_ELPM__");
   if (ArchHasELPMX(Arch))
diff --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index 731076d9754a9..155e0d812c95f 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -651,8 +651,19 @@ void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   // This is almost always required because otherwise avr-ld
   // will assume 'avr2' and warn about the program being larger
   // than the bare minimum supports.
-  if (Linker.find("avr-ld") != std::string::npos && FamilyName)
-    CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+  if (Linker.find("avr-ld") != std::string::npos && FamilyName) {
+    // Option to use mapped memory for modern devices with >32kB flash.
+    // This is the only option for modern devices with <= 32kB flash,
+    // but the larger default to a copy from flash to RAM (avr-ld version < 14)
+    // or map the highest 32kB to RAM (avr-ld version >= 14).
+    if (Args.hasFlag(options::OPT_mflmap, options::OPT_mrodata_in_ram, false)) {
+      CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName + "_flmap"));
+      CmdArgs.push_back(Args.MakeArgString(std::string("-u")));
+      CmdArgs.push_back(Args.MakeArgString(std::string("__do_flmap_init")));
+    }
+    else
+      CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+  }
 
   C.addCommand(std::make_unique<Command>(
       JA, *this, ResponseFileSupport::AtFileCurCP(), Args.MakeArgString(Linker),
diff --git a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
index ad8aa5717fb42..decfcc4b67f5d 100644
--- a/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
+++ b/llvm/lib/Target/AVR/AVRAsmPrinter.cpp
@@ -263,11 +263,17 @@ bool AVRAsmPrinter::doFinalization(Module &M) {
     auto *Section = cast<MCSectionELF>(TLOF.SectionForGlobal(&GO, TM));
     if (Section->getName().starts_with(".data"))
       NeedsCopyData = true;
-    else if (Section->getName().starts_with(".rodata") && SubTM->hasLPM())
+    else if (Section->getName().starts_with(".rodata") && SubTM->hasLPM()) {
       // AVRs that have a separate program memory (that's most AVRs) store
-      // .rodata sections in RAM.
-      NeedsCopyData = true;
-    else if (Section->getName().starts_with(".bss"))
+      // .rodata sections in RAM,
+      // but XMEGA3 family maps all flash in the data space.
+      // Invoking __do_copy_data with 0 bytes to copy will crash,
+      // so we let the loader handle this for newer devices.
+      if (!(SubTM->hasFeatureSetFamilyXMEGA2() ||
+            SubTM->hasFeatureSetFamilyXMEGA3() ||
+            SubTM->hasFeatureSetFamilyXMEGA4()))
+        NeedsCopyData = true;
+    } else if (Section->getName().starts_with(".bss"))
       NeedsClearBSS = true;
   }
 
diff --git a/llvm/lib/Target/AVR/AVRDevices.td b/llvm/lib/Target/AVR/AVRDevices.td
index ad760d7403573..0b6b0e3696e5f 100644
--- a/llvm/lib/Target/AVR/AVRDevices.td
+++ b/llvm/lib/Target/AVR/AVRDevices.td
@@ -49,6 +49,16 @@ def FeatureEIJMPCALL : SubtargetFeature<"eijmpcall", "HasEIJMPCALL", "true",
                                         "The device supports the "
                                         "`EIJMP`/`EICALL` instructions">;
 
+// The device supports `FLMAP` flash bank data mapping to the data space.
+def FeatureFLMAP : SubtargetFeature<"flmap", "HasFLMAP", "true",
+                                        "The device supports the "
+                                        "`FLMAP` mapping">;
+
+// The device maps all flash (<= 32kB) to the data space.
+def FeatureMAP : SubtargetFeature<"map", "HasMAP", "true",
+                                        "The device maps all flash (<= 32kB) "
+                                        "to the data space">;
+
 // The device supports `ADDI Rd, K`, `SUBI Rd, K`.
 def FeatureADDSUBIW : SubtargetFeature<"addsubiw", "HasADDSUBIW", "true",
                                        "Enable 16-bit register-immediate "
@@ -220,6 +230,7 @@ def FamilyXMEGA3 : Family<"xmega3",
                           [FamilyAVR0, FeatureLPM, FeatureIJMPCALL,
                            FeatureADDSUBIW, FeatureSRAM, FeatureJMPCALL,
                            FeatureMultiplication, FeatureMOVW, FeatureLPMX,
+                           FeatureMAP,
                            FeatureBREAK, FeatureLowByteFirst]>;
 
 def FamilyXMEGA4 : Family<"xmega4",
@@ -228,6 +239,7 @@ def FamilyXMEGA4 : Family<"xmega4",
                            FeatureMultiplication, FeatureMOVW, FeatureLPMX,
                            FeatureELPM, FeatureELPMX,
                            FeatureSPM, FeatureSPMX,
+                           FeatureFLMAP,
                            FeatureBREAK, FeatureLowByteFirst]>;
 
 def FamilyXMEGA : Family<"xmega",
@@ -275,9 +287,9 @@ def : Device<"avr5", FamilyAVR5, ELFArchAVR5>;
 def : Device<"avr51", FamilyAVR51, ELFArchAVR51>;
 def : Device<"avr6", FamilyAVR6, ELFArchAVR6>;
 def : Device<"avrxmega1", FamilyXMEGA, ELFArchXMEGA1>;
-def : Device<"avrxmega2", FamilyXMEGA, ELFArchXMEGA2>;
+def : Device<"avrxmega2", FamilyXMEGA2, ELFArchXMEGA2>;
 def : Device<"avrxmega3", FamilyXMEGA3, ELFArchXMEGA3>;
-def : Device<"avrxmega4", FamilyXMEGA, ELFArchXMEGA4>;
+def : Device<"avrxmega4", FamilyXMEGA4, ELFArchXMEGA4>;
 def : Device<"avrxmega5", FamilyXMEGA, ELFArchXMEGA5>;
 def : Device<"avrxmega6", FamilyXMEGA, ELFArchXMEGA6>;
 def : Device<"avrxmega7", FamilyXMEGA, ELFArchXMEGA7>;
@@ -596,26 +608,26 @@ def : Device<"atmega4809", FamilyXMEGA3, ELFArchXMEGA3>;
 
 // Additions from gcc 14:
 
-def : Device<"avr64da28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64da32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64da48", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64da64", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db48", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64db64", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd14", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd20", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64dd32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64du28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64du32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64ea28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64ea32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64ea48", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64sd28", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64sd32", FamilyXMEGA2, ELFArchXMEGA2>;
-def : Device<"avr64sd48", FamilyXMEGA2, ELFArchXMEGA2>;
+def : Device<"avr64da28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64da32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64da48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64da64", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64db64", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd14", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd20", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64dd32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64du28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64du32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64ea28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64ea32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64ea48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64sd28", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64sd32", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
+def : Device<"avr64sd48", FamilyXMEGA2, ELFArchXMEGA2, [FeatureFLMAP]>;
 
 def : Device<"avr16dd20", FamilyXMEGA3, ELFArchXMEGA3>;
 def : Device<"avr16dd28", FamilyXMEGA3, ELFArchXMEGA3>;

github-actions · 2025-06-28T21:44:52Z

✅ With the latest revision this PR passed the C/C++ code formatter.

tomtor · 2025-07-01T08:35:43Z

@benshi001 @Patryk27 Could you review this?

benshi001 · 2025-07-01T10:10:03Z

We should ask the owner of clang driver @MaskRay if a new option can be accepted.

benshi001 · 2025-07-01T10:29:17Z

@benshi001 @Patryk27 Could you review this?

As I have told you several days ago, each functional change requires unit tests, your PR involves

clang driver
clang builtin macro
avr backend codegen

Each part needs unit tests to show what are affected.

benshi001

unit tests are needed.

tomtor · 2025-07-01T11:02:50Z

@benshi001 @Patryk27 Could you review this?

As I have told you several days ago, each functional change requires unit tests, your PR involves
1. clang driver

2. clang builtin macro

3. avr backend codegen
Each part needs unit tests to show what are affected.

@benshi001 Can you point me to existing tests which vary the sub architecture of AVR? I would be happy to add to these tests the new architectures.

Patryk27 · 2025-07-01T13:42:13Z

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

+      if (!(SubTM->hasFeatureSetFamilyXMEGA2() ||
+            SubTM->hasFeatureSetFamilyXMEGA3() ||
+            SubTM->hasFeatureSetFamilyXMEGA4()))
+        NeedsCopyData = true;


Shouldn't we check the rodata-in-ram switch here?

No, for these newer devices __do_copy_data should never be pulled in. This is done by the ld spec-files and crt for these devices. See also #146537

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

Patryk27 · 2025-07-01T13:46:59Z

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

+      // but XMEGA3 family maps all flash in the data space.
+      // Invoking __do_copy_data with 0 bytes to copy will crash,
+      // so we let the loader handle this for newer devices.
+      if (!(SubTM->hasFeatureSetFamilyXMEGA2() ||


Could we check for FeatureFLMAP in here instead?

Feels like that's what it is supposed to do, but we use family as a proxy for the actual flag we're interested in.

See previous comment, the new clang driver switch is just a feature, to select an option for FLMAP devices. It is not critical and could be removed from this PR. Only the code fragment you refer to is essential to fix the issue.

All other changes (mainly adding some metadata for newer xmega devices) just gets the clang feature set more in sync with gcc for newer AVR devices.

Unless trivial (e.g. typo fix), "marked this conversation as resolved." should only be used by reviewers per suggestions on https://discourse.llvm.org/t/rfc-github-pr-resolve-conversation-button/73178

(Sorry, didn't see that tomtor is the author :) )

benshi001 · 2025-07-02T02:24:52Z

I do not think this PR is necessary, since there is no bug, I have explained in #146537.

tomtor · 2025-07-02T07:59:13Z

I do not think this PR is necessary, since there is no bug, I have explained in #146537.

You are right, so this PR is not a bug fix, but just adding features and a minor optimization.

tomtor · 2025-07-02T09:58:24Z

clang/lib/Driver/ToolChains/AVR.cpp

@@ -651,8 +651,19 @@ void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
  // This is almost always required because otherwise avr-ld
  // will assume 'avr2' and warn about the program being larger
  // than the bare minimum supports.
-  if (Linker.find("avr-ld") != std::string::npos && FamilyName)


@benshi001 If this line is relocated (moved a bit upwards) so that it is before handling user supplied linker arguments then a user could overwrite the default -m option.
As it is the user cannot overwrite the driver generated option.

benshi001 · 2025-07-02T10:03:17Z

I do not think this PR is necessary, since there is no bug, I have explained in #146537.

You are right, so this PR is not a bug fix, but just adding features and a minor optimization.

Currently I am focusing on bugfix and compatibility/substitutability with avr-gcc-7.3 (which is used by Arduino major version), and quite cautious about adding new features.

For the mflmap / mrodata-in-ram" / __do_flmap_init mentioned in your PR, I am not familiar with them. So I need more time to

Understand these features in newer AVR devices,
study how gcc and air-libc support them,
check if they affect stability of existing llvm-avr's functionalities.

What's more, I can only decide modifications to the AVR backend. If you want to add new clang options, you have to create a proposal at https://discourse.llvm.org, and let the clang reviewers to decide.

tomtor · 2025-07-02T10:06:25Z

@Benshi There is an alternative to new driver options: https://github.com/llvm/llvm-project/pull/146244/files#r2179646110

That would be a minimal change, and a general improvement giving users more options to tune the linking.

tomtor · 2025-07-02T10:10:14Z

Some background info:

https://gcc.gnu.org/gcc-14/changes.html#avr

MaskRay · 2025-07-02T16:51:21Z

clang/lib/Driver/ToolChains/AVR.cpp

+    // or map the highest 32kB to RAM (avr-ld version >= 14).
+    if (Args.hasFlag(options::OPT_mflmap, options::OPT_mrodata_in_ram, false)) {
+      CmdArgs.push_back(
+          Args.MakeArgString(std::string("-m") + *FamilyName + "_flmap"));


I am not familiar with these options. Are we sure this patch has listed all the effects?

gcc also seems to do something with the __do_copy_data symbol.

At the minimum, we need clang/test/Driver tests to check how a driver option influences the link options. For features, we need other tests (perhaps clang/test/CodeGen/AVR, which does not exist yet)

I agree, so I think I need more time to understand the whole picture what gcc did.

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 28, 2025

[AVR] Handle mapped RO data for newer devices

a08a6a0

tomtor force-pushed the flmap branch from 47e76c1 to a08a6a0 Compare June 28, 2025 21:52

benshi001 requested a review from MaskRay July 1, 2025 10:02

benshi001 requested review from Patryk27 and benshi001 July 1, 2025 10:30

benshi001 requested changes Jul 1, 2025

View reviewed changes

Patryk27 reviewed Jul 1, 2025

View reviewed changes

llvm/lib/Target/AVR/AVRAsmPrinter.cpp Show resolved Hide resolved

Patryk27 reviewed Jul 1, 2025

View reviewed changes

Fix comment

d9f0f25

tomtor commented Jul 2, 2025

View reviewed changes

tomtor mentioned this pull request Jul 2, 2025

[AVR] cannot find -lgcc when using Arduino avr-ld version #146668

Open

MaskRay reviewed Jul 2, 2025

View reviewed changes

[AVR] Handle flash RO data mapped to data space for newer devices #146244

Are you sure you want to change the base?

[AVR] Handle flash RO data mapped to data space for newer devices #146244

Uh oh!

Conversation

tomtor commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 28, 2025

Uh oh!

llvmbot commented Jun 28, 2025

Uh oh!

github-actions bot commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomtor commented Jul 1, 2025

Uh oh!

benshi001 commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benshi001 commented Jul 1, 2025

Uh oh!

benshi001 left a comment

Choose a reason for hiding this comment

Uh oh!

tomtor commented Jul 1, 2025

Uh oh!

Patryk27 Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

tomtor Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Patryk27 Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomtor Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaskRay Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

MaskRay Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

benshi001 commented Jul 2, 2025

Uh oh!

tomtor commented Jul 2, 2025

Uh oh!

tomtor Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

benshi001 commented Jul 2, 2025

Uh oh!

tomtor commented Jul 2, 2025

Uh oh!

tomtor commented Jul 2, 2025

Uh oh!

MaskRay Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benshi001 Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomtor commented Jun 28, 2025 •

edited

Loading

github-actions bot commented Jun 28, 2025 •

edited

Loading

benshi001 commented Jul 1, 2025 •

edited

Loading

Patryk27 Jul 1, 2025 •

edited

Loading

tomtor Jul 1, 2025 •

edited

Loading

MaskRay Jul 2, 2025 •

edited

Loading