[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