[PATCH] D14199: [compiler-rt] [tsan] Unify aarch64 mapping
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 4 04:11:38 PST 2015
dvyukov added inline comments.
================
Comment at: lib/tsan/rtl/tsan_platform.h:153
@@ +152,3 @@
+
+extern uptr kLoAppMemBeg;
+extern uptr kLoAppMemEnd;
----------------
You introduce both indirect functions and these non-const variables. This looks excessive. Let's do one or another. I would prefer global variables as it looks less intrusive.
You can do something along the lines of:
Here declare the consts:
extern uptr kLoAppMemBeg;
...
And then in InitializePlatformEarly initialize them:
kLoAppMemEnd = vma_39_42(0x0000400000ull, 0x00000400000ull);
And that's it.
================
Comment at: lib/tsan/rtl/tsan_platform.h:169
@@ -148,1 +168,3 @@
+
+#define __TSAN_NOINLINE_FUNCTIONS 1
#endif
----------------
All identifiers with double underscores are reserved for language implementation. TSAN_ is a fine prefix, and also consistent with all macros in the codebase.
================
Comment at: lib/tsan/rtl/tsan_platform.h:383
@@ -342,2 +382,3 @@
void InitializePlatform();
+void InitializePlatformSpecificModules();
void FlushShadowMemory();
----------------
Please rename this to InitializePlatformEarly.
I don't understand what is "Modules" (dynamic libraries?). And "Specific" is excessive, "Platform" already implies platform-specific stuff.
http://reviews.llvm.org/D14199
More information about the llvm-commits
mailing list