[PATCH] D65863: [ARM] Add support for the s,j,x,N,O inline asm constraints

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: clang/lib/Basic/Targets/ARM.cpp:904
+  case 'j': // An immediate integer between 0 and 65535 (valid for MOVW)
+    if (CPUAttr.equals("6T2") ||
+        ArchVersion >= 7) // only available in ARMv6T2 and above
----------------
I would hoist the comment above the `if` as that results in a more readable condition.


================
Comment at: clang/lib/Basic/Targets/ARM.cpp:938
+    // Thumb1: An immediate integer which is a multiple of 4 between 0 and 1020.
+    Info.setRequiresImmediate();
     return true;
----------------
Can we leave this as a FIXME?  This needs additional validation on the input.


================
Comment at: clang/lib/Basic/Targets/ARM.cpp:941
+  case 'N':
+    if (isThumb() && !supportsThumb2()) // Thumb1 only
+    {
----------------
Please hoist the comment above the `if`.


================
Comment at: clang/lib/Basic/Targets/ARM.cpp:948
+  case 'O':
+    if (isThumb() && !supportsThumb2()) // Thumb1 only
+    {
----------------
Similar


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65863/new/

https://reviews.llvm.org/D65863