[PATCH] D43155: [asan] Add "arm64" into the list of 64-bit architectures

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

The patch is general looks good. Just a minor nit and a potential idea for a different implementation.

Does this patch change which ASan tests get run on arm64?



================
Comment at: test/asan/CMakeLists.txt:21
 macro(get_bits_for_arch arch bits)
-  if (${arch} MATCHES "i386|arm|mips|mipsel")
-    set(${bits} 32)
-  elseif (${arch} MATCHES "x86_64|powerpc64|powerpc64le|aarch64|mips64|mips64el|s390x")
+  if (${arch} MATCHES "x86_64|powerpc64|powerpc64le|aarch64|arm64|mips64|mips64el|s390x")
     set(${bits} 64)
----------------
I wonder if we should avoid using `MATCHES` and use `STREQUAL` instead. This would require listing each supported architecture (i.e. all variants would need to be specified) explicitly. This would avoid accidentally matching architectures in the future. This would bloat the code a bit though.

This might also be a bit painful for architecture variants that are supported out of tree because they'd now have to patch this macro whereas before it "just worked".


================
Comment at: test/asan/CMakeLists.txt:23
     set(${bits} 64)
+  elseif (${arch} MATCHES "i386|arm|mips|mipsel")
+    set(${bits} 32)
----------------
The arm regex should probably be `^armv7` 


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43155