[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 additional architecture info for devices with an flmap register (xmega2 and xmega4 with >= 64kB flash) and add a gcc compatible preprocessor symbol.

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.

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

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

@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.

I can remove the new linker options. A user could use -T and additional linker options if needed.

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: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

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.

tomtor · 2025-07-14T12:01:41Z

I removed the new flags, these can be simulated with -T and other linker flags by a user.

tomtor · 2025-07-14T12:18:00Z

I did not request a review from code owners? Did I hit a button by accident? EDIT: Had some rebase issues and for a while some commits from others in this PR. I suppose that triggered this. Did a force push to clear it

benshi001 · 2025-08-05T00:57:38Z

Currently I put support of new features at a low priority, since arduino still uses gcc-7.3 as its main tool chain.

Patryk27 · 2025-08-05T08:14:59Z

Sure, just wanted to note that it LGTM, but feel free to leave it for another time.

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

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 Outdated Show resolved Hide resolved

Patryk27 reviewed Jul 1, 2025

View reviewed changes

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

Patryk27 reviewed Jul 1, 2025

View reviewed changes

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

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

Closed

MaskRay reviewed Jul 2, 2025

View reviewed changes

tomtor requested review from DeinAlptraum, QuietMisdreavus, aaupov, cyndyishida and daniel-grumberg as code owners July 14, 2025 11:25

tomtor requested review from a team, Endilll, ayermolo, bcardosolopes, lanza, maksfb, paschalis-mpeis, rafaelauler and yota9 as code owners July 14, 2025 11:25

tomtor added 2 commits July 14, 2025 13:30

[AVR] Handle mapped RO data for newer devices

137c182

Fix comment

5c32a48

tomtor force-pushed the flmap branch 2 times, most recently from 84065da to 5c32a48 Compare July 14, 2025 11:44

remove new flags

d5d09f8

QuietMisdreavus removed their request for review July 14, 2025 15:21

Patryk27 approved these changes Aug 4, 2025

View reviewed changes

Endilll removed their request for review August 5, 2025 12:45

[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!

Uh oh!

Uh oh!

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!

tomtor Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

benshi001 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!

tomtor commented Jul 14, 2025

Uh oh!

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

Uh oh!

Uh oh!

benshi001 commented Aug 5, 2025

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tomtor commented Jun 28, 2025 •

edited

Loading

github-actions bot commented Jun 28, 2025 •

edited

Loading

benshi001 commented Jul 1, 2025 •

edited

Loading

MaskRay Jul 2, 2025 •

edited

Loading

tomtor commented Jul 14, 2025 •

edited

Loading

Patryk27 commented Aug 5, 2025 •

edited

Loading