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

[Dan Liew via Phabricator via llvm-commits]

delcypher accepted this revision.
delcypher added inline comments.
This revision is now accepted and ready to land.


================
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)
----------------
kubamracek wrote:
> delcypher wrote:
> > 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".
> Generally, I agree, but I don't want to do it in this patch. Right now, let's just fix the obvious mistake of matching "arm64" and "mips64" as 32-bit architectures.
Sure.


================
Comment at: test/asan/CMakeLists.txt:23
     set(${bits} 64)
+  elseif (${arch} MATCHES "i386|arm|mips|mipsel")
+    set(${bits} 32)
----------------
kubamracek wrote:
> delcypher wrote:
> > The arm regex should probably be `^armv7` 
> There's also `armv6` and probably other `arm` variants. Let's keep this as "arm" to avoid breaking others.
I think the `^armv6` and `^armv7` regexes would capture all those variants but I won't insist on this.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43155