From 171f793c1e2d93ba0b78fc4b2b0625715f03e115 Mon Sep 17 00:00:00 2001
From: Ayke van Laethem <aykevanlaethem@gmail.com>
Date: Sat, 31 Oct 2020 20:40:20 +0100
Subject: [PATCH] avr: properly support the .rodata section

Unfortunately, the .rodata section can't be stored in flash. Instead, an
explicit .progmem section should be used, which is supported in LLVM as
address space 1 but not exposed to normal programs.

Eventually a pass should be written that converts trivial const globals
of which all loads are visible to be in addrspace 1, to get the benefits
of storing those globals directly in ROM.
---
 builder/build.go                            | 12 ------------
 targets/avr.ld                              |  6 ++++--
 transform/globals.go                        | 13 -------------
 transform/globals_test.go                   |  7 -------
 transform/testdata/globals-non-const.ll     |  5 -----
 transform/testdata/globals-non-const.out.ll |  5 -----
 6 files changed, 4 insertions(+), 44 deletions(-)
 delete mode 100644 transform/testdata/globals-non-const.ll
 delete mode 100644 transform/testdata/globals-non-const.out.ll

diff --git a/builder/build.go b/builder/build.go
index 11ad6b97..3e7ce62a 100644
--- a/builder/build.go
+++ b/builder/build.go
@@ -117,18 +117,6 @@ func Build(pkgName, outpath string, config *compileopts.Config, action func(Buil
 		return errors.New("verification failure after LLVM optimization passes")
 	}
 
-	// On the AVR, pointers can point either to flash or to RAM, but we don't
-	// know. As a temporary fix, load all global variables in RAM.
-	// In the future, there should be a compiler pass that determines which
-	// pointers are flash and which are in RAM so that pointers can have a
-	// correct address space parameter (address space 1 is for flash).
-	if strings.HasPrefix(config.Triple(), "avr") {
-		transform.NonConstGlobals(mod)
-		if err := llvm.VerifyModule(mod, llvm.PrintMessageAction); err != nil {
-			return errors.New("verification error after making all globals non-constant on AVR")
-		}
-	}
-
 	// LLVM 11 by default tries to emit tail calls (even with the target feature
 	// disabled) unless it is explicitly disabled with a function attribute.
 	// This is a problem, as it tries to emit them and prints an error when it
diff --git a/targets/avr.ld b/targets/avr.ld
index a7c2d90c..720465ca 100644
--- a/targets/avr.ld
+++ b/targets/avr.ld
@@ -13,8 +13,8 @@ SECTIONS
         KEEP(*(.text.__vector_RESET))
         KEEP(*(.text.main)) /* main must follow the reset handler */
         *(.text.*)
-        *(.rodata)
-        *(.rodata.*)
+        *(.progmem)
+        *(.progmem.*)
     }
 
     .stack (NOLOAD) :
@@ -28,6 +28,8 @@ SECTIONS
     .data :
     {
         _sdata = .;        /* used by startup code */
+        *(.rodata)
+        *(.rodata.*)
         *(.data)
         *(.data*)
         _edata = .;        /* used by startup code */
diff --git a/transform/globals.go b/transform/globals.go
index 7a296835..2d0349e9 100644
--- a/transform/globals.go
+++ b/transform/globals.go
@@ -19,19 +19,6 @@ func ApplyFunctionSections(mod llvm.Module) {
 	}
 }
 
-// NonConstGlobals turns all global constants into global variables. This works
-// around a limitation on Harvard architectures (e.g. AVR), where constant and
-// non-constant pointers point to a different address space. Normal pointer
-// behavior is restored by using the data space only, at the cost of RAM for
-// constant global variables.
-func NonConstGlobals(mod llvm.Module) {
-	global := mod.FirstGlobal()
-	for !global.IsNil() {
-		global.SetGlobalConstant(false)
-		global = llvm.NextGlobal(global)
-	}
-}
-
 // DisableTailCalls adds the "disable-tail-calls"="true" function attribute to
 // all functions. This may be necessary, in particular to avoid an error with
 // WebAssembly in LLVM 11.
diff --git a/transform/globals_test.go b/transform/globals_test.go
index 14c77e6a..0dcb0608 100644
--- a/transform/globals_test.go
+++ b/transform/globals_test.go
@@ -12,10 +12,3 @@ func TestApplyFunctionSections(t *testing.T) {
 		ApplyFunctionSections(mod)
 	})
 }
-
-func TestNonConstGlobals(t *testing.T) {
-	t.Parallel()
-	testTransform(t, "testdata/globals-non-const", func(mod llvm.Module) {
-		NonConstGlobals(mod)
-	})
-}
diff --git a/transform/testdata/globals-non-const.ll b/transform/testdata/globals-non-const.ll
deleted file mode 100644
index 5f3c5846..00000000
--- a/transform/testdata/globals-non-const.ll
+++ /dev/null
@@ -1,5 +0,0 @@
-target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
-target triple = "armv7em-none-eabi"
-
-@globalIntConst = constant i32 3
-@globalIntVar = global i32 5
diff --git a/transform/testdata/globals-non-const.out.ll b/transform/testdata/globals-non-const.out.ll
deleted file mode 100644
index 33b577eb..00000000
--- a/transform/testdata/globals-non-const.out.ll
+++ /dev/null
@@ -1,5 +0,0 @@
-target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
-target triple = "armv7em-none-eabi"
-
-@globalIntConst = global i32 3
-@globalIntVar = global i32 5