From 5985f7efb519a8111ffd1777bfa79a24e3abf5bf Mon Sep 17 00:00:00 2001
From: occheung <dc@m-labs.hk>
Date: Mon, 30 Aug 2021 10:53:42 +0800
Subject: [PATCH] syscall: lower nowrite to inaccessiblememonly

In the origin implementation, the `nowrite` flag literally means not writing memory at all.
Due to the usage of flags on certain functions, it results in the same issues found in artiq-zynq after optimization passes. (https://git.m-labs.hk/M-Labs/artiq-zynq/issues/119)
A fix wrote by @dnadlinger can resolve this issue. (https://github.com/dnadlinger/artiq/commit/c1e46cc7c8d9b2a4b58a45d6f5dabe65a127fb61)
---
 artiq/compiler/transforms/llvm_ir_generator.py | 15 ++-------------
 artiq/compiler/types.py                        |  2 +-
 artiq/coredevice/cache.py                      |  4 ++--
 artiq/test/lit/embedding/syscall_flags.py      |  4 ++--
 4 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/artiq/compiler/transforms/llvm_ir_generator.py b/artiq/compiler/transforms/llvm_ir_generator.py
index 180560f47..970ade340 100644
--- a/artiq/compiler/transforms/llvm_ir_generator.py
+++ b/artiq/compiler/transforms/llvm_ir_generator.py
@@ -84,14 +84,6 @@ class LLVMIRGenerator:
         self.llobject_map = {}
         self.phis = []
         self.empty_metadata = self.llmodule.add_metadata([])
-        self.tbaa_tree = self.llmodule.add_metadata([
-            ll.MetaDataString(self.llmodule, "ARTIQ TBAA")
-        ])
-        self.tbaa_nowrite_call = self.llmodule.add_metadata([
-            ll.MetaDataString(self.llmodule, "ref-only function call"),
-            self.tbaa_tree,
-            ll.Constant(lli64, 1)
-        ])
         self.quote_fail_msg = None
 
     def needs_sret(self, lltyp, may_be_large=True):
@@ -1207,6 +1199,8 @@ class LLVMIRGenerator:
                 llfun.args[i].add_attribute('byval')
             if 'nounwind' in insn.target_function().type.flags:
                 llfun.attributes.add('nounwind')
+            if 'nowrite' in insn.target_function().type.flags:
+                llfun.attributes.add('inaccessiblememonly')
 
         return llfun, list(llargs)
 
@@ -1364,11 +1358,6 @@ class LLVMIRGenerator:
                 # {} elsewhere.
                 llresult = ll.Constant(llunit, [])
 
-            # Never add TBAA nowrite metadata to a functon with sret!
-            # This leads to miscompilations.
-            if types.is_external_function(functiontyp) and 'nowrite' in functiontyp.flags:
-                llcall.set_metadata('tbaa', self.tbaa_nowrite_call)
-
         return llresult
 
     def process_Invoke(self, insn):
diff --git a/artiq/compiler/types.py b/artiq/compiler/types.py
index b5a220aeb..1d9336b4d 100644
--- a/artiq/compiler/types.py
+++ b/artiq/compiler/types.py
@@ -304,7 +304,7 @@ class TExternalFunction(TFunction):
         mangling rules).
     :ivar flags: (set of str) function flags.
         Flag ``nounwind`` means the function never raises an exception.
-        Flag ``nowrite`` means the function never writes any memory
+        Flag ``nowrite`` means the function never accesses any memory
         that the ARTIQ Python code can observe.
     :ivar broadcast_across_arrays: (bool)
         If True, the function is transparently applied element-wise when called
diff --git a/artiq/coredevice/cache.py b/artiq/coredevice/cache.py
index 7b01e96b9..6c222bb6f 100644
--- a/artiq/coredevice/cache.py
+++ b/artiq/coredevice/cache.py
@@ -2,11 +2,11 @@ from artiq.language.core import *
 from artiq.language.types import *
 
 
-@syscall(flags={"nounwind", "nowrite"})
+@syscall(flags={"nounwind"})
 def cache_get(key: TStr) -> TList(TInt32):
     raise NotImplementedError("syscall not simulated")
 
-@syscall(flags={"nowrite"})
+@syscall
 def cache_put(key: TStr, value: TList(TInt32)) -> TNone:
     raise NotImplementedError("syscall not simulated")
 
diff --git a/artiq/test/lit/embedding/syscall_flags.py b/artiq/test/lit/embedding/syscall_flags.py
index 7d2a10341..f8c618c3f 100644
--- a/artiq/test/lit/embedding/syscall_flags.py
+++ b/artiq/test/lit/embedding/syscall_flags.py
@@ -4,9 +4,9 @@
 from artiq.language.core import *
 from artiq.language.types import *
 
-# CHECK: call void @foo\(\)(, !dbg !\d+)?, !tbaa !\d+
+# CHECK: call void @foo\(\)(, !dbg !\d+)?
 
-# CHECK-L: ; Function Attrs: nounwind
+# CHECK-L: ; Function Attrs: inaccessiblememonly nounwind
 # CHECK-NEXT-L: declare void @foo()
 
 @syscall(flags={"nounwind", "nowrite"})