[clang] 88b7b76 - [AVR][clang] Pass the address of the data section to the linker for ATmega328

[Dylan McKay via cfe-commits]

Author: Dylan McKay
Date: 2020-10-29T06:35:15+13:00
New Revision: 88b7b76a0b2365fe4ea9f686c6346667bfe48488

URL: https://github.com/llvm/llvm-project/commit/88b7b76a0b2365fe4ea9f686c6346667bfe48488
DIFF: https://github.com/llvm/llvm-project/commit/88b7b76a0b2365fe4ea9f686c6346667bfe48488.diff

LOG: [AVR][clang] Pass the address of the data section to the linker for ATmega328

This patch modifies the Clang AVR toolchain so that it always passes
the '-Tdata=0x800100' to the linker for ATmega328 devices. This matches
AVR-GCC behaviour, and also corresponds to the address of the start of
the data section in data space according to the ATmega328 datasheet.

Without this, clang does not produce a valid ATmega328 binary.

When targeting all non-ATmega328 chips, a warning will be emitted due to
the fact that proper handling for the chips data section address is
not yet implemented.

I've held off adding other microcontrollers for now, mostly because the
AVR toolchain logic is smeared across LLVM core TableGen files, and two Clang
libraries. The 'family detection' logic is also only implemented for
ATmega328 at the moment, for similar reasons.

In the future, I aim to write an RFC to llvm-dev to find a better way
for LLVM to expose target-specific details such as these to compiler
frontends.

Differential Revision: https://reviews.llvm.org/D86629

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticDriverKinds.td
    clang/lib/Driver/ToolChains/AVR.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 5336e7648001..a45d9e8ee788 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -44,6 +44,10 @@ def warn_drv_avr_libc_not_found: Warning<
 def warn_drv_avr_family_linking_stdlibs_not_implemented: Warning<
   "support for linking stdlibs for microcontroller '%0' is not implemented">,
   InGroup<AVRRtlibLinkingQuirks>;
+def warn_drv_avr_linker_section_addresses_not_implemented: Warning<
+  "support for passing the data section address to the linker for "
+  "microcontroller '%0' is not implemented">,
+  InGroup<AVRRtlibLinkingQuirks>;
 def warn_drv_avr_stdlib_not_linked: Warning<
   "standard library not linked and so no interrupt vector table or "
   "compiler runtime routines will be linked">,

diff  --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index 02b745c6a205..dfe3561cce46 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/SubtargetFeature.h"
@@ -29,13 +30,20 @@ namespace {
 
 // TODO: Consider merging this into the AVR device table
 // array in Targets/AVR.cpp.
-llvm::Optional<StringRef> GetMcuFamilyName(StringRef MCU) {
+llvm::Optional<StringRef> GetMCUFamilyName(StringRef MCU) {
   return llvm::StringSwitch<llvm::Optional<StringRef>>(MCU)
       .Case("atmega328", Optional<StringRef>("avr5"))
       .Case("atmega328p", Optional<StringRef>("avr5"))
       .Default(Optional<StringRef>());
 }
 
+llvm::Optional<unsigned> GetMCUSectionAddressData(StringRef MCU) {
+  return llvm::StringSwitch<llvm::Optional<unsigned>>(MCU)
+      .Case("atmega328", Optional<unsigned>(0x800100))
+      .Case("atmega328p", Optional<unsigned>(0x800100))
+      .Default(Optional<unsigned>());
+}
+
 const StringRef PossibleAVRLibcLocations[] = {
     "/usr/avr",
     "/usr/lib/avr",
@@ -59,7 +67,7 @@ AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
       // We cannot link any standard libraries without an MCU specified.
       D.Diag(diag::warn_drv_avr_mcu_not_specified);
     } else {
-      Optional<StringRef> FamilyName = GetMcuFamilyName(CPU);
+      Optional<StringRef> FamilyName = GetMCUFamilyName(CPU);
       Optional<std::string> AVRLibcRoot = findAVRLibcInstallation();
 
       if (!FamilyName.hasValue()) {
@@ -102,7 +110,8 @@ void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                const char *LinkingOutput) const {
   // Compute information about the target AVR.
   std::string CPU = getCPUName(Args, getToolChain().getTriple());
-  llvm::Optional<StringRef> FamilyName = GetMcuFamilyName(CPU);
+  llvm::Optional<StringRef> FamilyName = GetMCUFamilyName(CPU);
+  llvm::Optional<unsigned> SectionAddressData = GetMCUSectionAddressData(CPU);
 
   std::string Linker = getToolChain().GetProgramPath(getShortName());
   ArgStringList CmdArgs;
@@ -118,6 +127,17 @@ void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   getToolChain().AddFilePathLibArgs(Args, CmdArgs);
 
+  if (SectionAddressData.hasValue()) {
+    std::string DataSectionArg = std::string("-Tdata=0x") +
+                                 llvm::utohexstr(SectionAddressData.getValue());
+    CmdArgs.push_back(Args.MakeArgString(DataSectionArg));
+  } else {
+    // We do not have an entry for this CPU in the address mapping table yet.
+    getToolChain().getDriver().Diag(
+        diag::warn_drv_avr_linker_section_addresses_not_implemented)
+        << CPU;
+  }
+
   // If the family name is known, we can link with the device-specific libgcc.
   // Without it, libgcc will simply not be linked. This matches avr-gcc
   // behavior.