[PATCH] D52167: [compiler-rt][TSan] Add TSan runtime support for Go on linux-aarch64.

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/go/buildgo.sh:144
+if [ "$SUFFIX" = "linux_arm64" ]; then
+	FLAGS=" -I../rtl -I../.. -I../../sanitizer_common -I../../../include -std=c++11 -Wall -fno-exceptions -fno-rtti -DSANITIZER_GO=1 -DSANITIZER_DEADLOCK_DETECTOR_VERSION=2 $OSCFLAGS"
+fi
----------------
Please don't duplicate all flags, and instead add arm64-specific flags below, like it is done for amd64 and ppc64le.


================
Comment at: lib/tsan/go/buildgo.sh:164
+if [ "$SUFFIX" = "linux_arm64" ]; then
+	$CC $OSCFLAGS test.c $DIR/race_$SUFFIX.syso -g -o $DIR/test $OSLDFLAGS $LDFLAGS
+else
----------------
Please don't duplicate whole command line. If -m64 breaks arm64, create ARCHCFLAGS, populate it with -m64 for amd64 and ppc64le, add ARCHCFLAGS to FLAGS and here.


================
Comment at: lib/tsan/rtl/tsan_platform.h:713
   switch (vmaSize) {
+#if !SANITIZER_GO
     case 39: return IsAppMemImpl<Mapping39>(mem);
----------------
Do we plan to support more vma sizes? If not, then it can make sense to use the generic `IsAppMemImpl<Mapping>(mem)` version without the switch, because it's still a load and check of the global var per memory access.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52167