[PATCH] D31220: [builtins][ARM] Select correct code fragments when compiling for Thumb1/Thum2/ARM ISA

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Just a few comments on the ARM specific terminology. I think it would be better to match the existing documentation such as using state for ARM/Thumb and not mode.



================
Comment at: lib/builtins/assembly.h:102
+#if defined(__thumb2__) || defined(__thumb__)
+#define DEFINE_CODE_MODE .thumb
+#define DECLARE_FUNC_ENCODING    .thumb_func SEPARATOR
----------------
I think DEFINE_CODE_STATE would be a better choice of name. This would match the ARM documentation more closely. ARM and Thumb are instruction states, this is distinct from modes which are User, IRQ, FIQ etc.
  


================
Comment at: lib/builtins/assembly.h:110
 #else
+#define USE_THUMB_1
+#define IT(cond)
----------------
I think it would be better to say USE_THUMB rather than USE_THUMB_1.

Strictly speaking there isn't such a thing as Thumb 1. There is just ARM and Thumb states and Thumb 2 is a set of extensions called "Thumb 2 technology".




================
Comment at: test/asan/CMakeLists.txt:10
 set(SHADOW_MAPPING_UNRELIABLE FALSE)
-if(OS_NAME MATCHES "Windows" AND CMAKE_SIZEOF_VOID_P EQUAL 8 AND
-    ${CMAKE_SYSTEM_VERSION} LESS 6.2)
----------------
I'm in no position to judge whether removing this is the right thing to do, but I can't immediately see how it is related to this patch?


https://reviews.llvm.org/D31220