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

[dcandler]

Summary

A number of inline assembly constraints are currently supported by LLVM, but rejected as invalid by Clang:

Target independent constraints:

s: An integer constant, but allowing only relocatable values

ARM specific constraints:

j: An immediate integer between 0 and 65535 (valid for MOVW)
x: A 32, 64, or 128-bit floating-point/SIMD register: s0-s15, d0-d7, or q0-q3
N: An immediate integer between 0 and 31 (Thumb1 only)
O: An immediate integer which is a multiple of 4 between -508 and 508. (Thumb1 only)

This patch adds support to Clang for the missing constraints along with some checks to ensure that the constraints are used with the correct target and Thumb mode, and that immediates are within valid ranges (at least where possible). The constraints are already implemented in LLVM, but just a couple of minor corrections to checks (V8M Baseline includes MOVW so should work with 'j', 'N' and 'O' shouldn't be valid in Thumb2) so that Clang and LLVM are in line with each other and the documentation.

[compnerd]

clang/lib/Basic/Targets/ARM.cpp

904 ↗

(On Diff #213852)

I would hoist the comment above the if as that results in a more readable condition.

938 ↗

(On Diff #213852)

Can we leave this as a FIXME? This needs additional validation on the input.

941 ↗

(On Diff #213852)

Please hoist the comment above the if.

948 ↗

(On Diff #213852)

Similar

[dcandler]

clang/lib/Basic/Targets/ARM.cpp

938 ↗

(On Diff #213852)

I think it's not just the M constraint that requires additional validation. Most of these immediate constraints require values that can fit in specific encodings to be valid, or have properties like being a multiple of a number, but setRequiresImmediate at present can only check against a min/max or exact values.

[dcandler]

Adjusted the formatting on some comment lines, and added FIXMEs for all the constraints that require additional validation to clarify what is still needed and where.

[dcandler]

Ping. @compnerd any other changes before this could be accepted?

[ostannard]

LGTM with one small change.

clang/test/Sema/arm_inline_asm_constraints.c

47 ↗

(On Diff #214205)

Copy-paste error, should be "between 0 and 255".

cfe/trunk/lib/Basic/Targets/ARM.cpp

ArrayRef<TargetInfo::GCCRegAlias> ARMTargetInfo::getGCCRegAliases() const {
   return llvm::makeArrayRef(GCCRegAliases);
 }
 
 bool ARMTargetInfo::validateAsmConstraint(
     const char *&Name, TargetInfo::ConstraintInfo &Info) const {
   switch (*Name) {
   default:
     break;
-  case 'l': // r0-r7
-  case 'h': // r8-r15
-  case 't': // VFP Floating point register single precision
-  case 'w': // VFP Floating point register double precision
+  case 'l': // r0-r7 if thumb, r0-r15 if ARM
     Info.setAllowsRegister();
     return true;
+  case 'h': // r8-r15, thumb only
+    if (isThumb()) {
+      Info.setAllowsRegister();
+      return true;
+    }
+    break;
+  case 's': // An integer constant, but allowing only relocatable values.
+    return true;
+  case 't': // s0-s31, d0-d31, or q0-q15
+  case 'w': // s0-s15, d0-d7, or q0-q3
+  case 'x': // s0-s31, d0-d15, or q0-q7
+    Info.setAllowsRegister();
+    return true;
+  case 'j': // An immediate integer between 0 and 65535 (valid for MOVW)
+    // only available in ARMv6T2 and above
+    if (CPUAttr.equals("6T2") || ArchVersion >= 7) {
+      Info.setRequiresImmediate(0, 65535);
+      return true;
+    }
+    break;
   case 'I':
+    if (isThumb()) {
+      if (!supportsThumb2())
+        Info.setRequiresImmediate(0, 255);
+      else
+        // FIXME: should check if immediate value would be valid for a Thumb2
+        // data-processing instruction
+        Info.setRequiresImmediate();
+    } else
+      // FIXME: should check if immediate value would be valid for an ARM
+      // data-processing instruction
+      Info.setRequiresImmediate();
+    return true;
   case 'J':
+    if (isThumb() && !supportsThumb2())
+      Info.setRequiresImmediate(-255, -1);
+    else
+      Info.setRequiresImmediate(-4095, 4095);
+    return true;
   case 'K':
+    if (isThumb()) {
+      if (!supportsThumb2())
+        // FIXME: should check if immediate value can be obtained from shifting
+        // a value between 0 and 255 left by any amount
+        Info.setRequiresImmediate();
+      else
+        // FIXME: should check if immediate value would be valid for a Thumb2
+        // data-processing instruction when inverted
+        Info.setRequiresImmediate();
+    } else
+      // FIXME: should check if immediate value would be valid for an ARM
+      // data-processing instruction when inverted
+      Info.setRequiresImmediate();
+    return true;
   case 'L':
+    if (isThumb()) {
+      if (!supportsThumb2())
+        Info.setRequiresImmediate(-7, 7);
+      else
+        // FIXME: should check if immediate value would be valid for a Thumb2
+        // data-processing instruction when negated
+        Info.setRequiresImmediate();
+    } else
+      // FIXME: should check if immediate value  would be valid for an ARM
+      // data-processing instruction when negated
+      Info.setRequiresImmediate();
+    return true;
   case 'M':
-    // FIXME
+    if (isThumb() && !supportsThumb2())
+      // FIXME: should check if immediate value is a multiple of 4 between 0 and
+      // 1020
+      Info.setRequiresImmediate();
+    else
+      // FIXME: should check if immediate value is a power of two or a integer
+      // between 0 and 32
+      Info.setRequiresImmediate();
+    return true;
+  case 'N':
+    // Thumb1 only
+    if (isThumb() && !supportsThumb2()) {
+      Info.setRequiresImmediate(0, 31);
+      return true;
+    }
+    break;
+  case 'O':
+    // Thumb1 only
+    if (isThumb() && !supportsThumb2()) {
+      // FIXME: should check if immediate value is a multiple of 4 between -508
+      // and 508
+      Info.setRequiresImmediate();
       return true;
+    }
+    break;
   case 'Q': // A memory address that is a single base register.
     Info.setAllowsMemory();
     return true;
   case 'T':
     switch (Name[1]) {
     default:
       break;
     case 'e': // Even general-purpose register

cfe/trunk/test/Sema/arm_inline_asm_constraints.c

+// REQUIRES: arm-registered-target
+
+// RUN: %clang_cc1 -triple armv6 -verify=arm6 %s
+// RUN: %clang_cc1 -triple armv7 -verify=arm7 %s
+// RUN: %clang_cc1 -triple thumbv6 -verify=thumb1 %s
+// RUN: %clang_cc1 -triple thumbv7 -verify=thumb2 %s
+
+// j: An immediate integer between 0 and 65535 (valid for MOVW) (ARM/Thumb2)
+int test_j(int i) {
+  int res;
+  __asm("movw %0, %1;"
+        : [ result ] "=r"(res)
+        : [ constant ] "j"(-1), [ input ] "r"(i)
+        :);
+  // arm6-error@13 {{invalid input constraint 'j' in asm}}
+  // arm7-error@13 {{value '-1' out of range for constraint 'j'}}
+  // thumb1-error@13 {{invalid input constraint 'j' in asm}}
+  // thumb2-error@13 {{value '-1' out of range for constraint 'j'}}
+  __asm("movw %0, %1;"
+        : [ result ] "=r"(res)
+        : [ constant ] "j"(0), [ input ] "r"(i)
+        :);
+  // arm6-error@21 {{invalid input constraint 'j' in asm}}
+  // arm7-no-error
+  // thumb1-error@21 {{invalid input constraint 'j' in asm}}
+  // thumb2-no-error
+  __asm("movw %0, %1;"
+        : [ result ] "=r"(res)
+        : [ constant ] "j"(65535), [ input ] "r"(i)
+        :);
+  // arm6-error@29 {{invalid input constraint 'j' in asm}}
+  // arm7-no-error
+  // thumb1-error@29 {{invalid input constraint 'j' in asm}}
+  // thumb2-no-error
+  __asm("movw %0, %1;"
+        : [ result ] "=r"(res)
+        : [ constant ] "j"(65536), [ input ] "r"(i)
+        :);
+  // arm6-error@37 {{invalid input constraint 'j' in asm}}
+  // arm7-error@37 {{value '65536' out of range for constraint 'j'}}
+  // thumb1-error@37 {{invalid input constraint 'j' in asm}}
+  // thumb2-error@37 {{value '65536' out of range for constraint 'j'}}
+  return res;
+}
+
+// I: An immediate integer valid for a data-processing instruction. (ARM/Thumb2)
+//    An immediate integer between 0 and 255. (Thumb1)
+int test_I(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "I"(-1), [ input ] "r"(i)
+      :); // thumb1-error@53 {{value '-1' out of range for constraint 'I'}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "I"(0), [ input ] "r"(i)
+      :); // No errors expected.
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "I"(255), [ input ] "r"(i)
+      :); // No errors expected.
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "I"(256), [ input ] "r"(i)
+      :); // thumb1-error@68 {{value '256' out of range for constraint 'I'}}
+  return res;
+}
+
+// J: An immediate integer between -4095 and 4095. (ARM/Thumb2)
+//    An immediate integer between -255 and -1. (Thumb1)
+int test_J(int i) {
+  int res;
+  __asm(
+      "movw %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(-4096), [ input ] "r"(i)
+      :);
+  // arm6-error@80 {{value '-4096' out of range for constraint 'J'}}
+  // arm7-error@80 {{value '-4096' out of range for constraint 'J'}}
+  // thumb1-error@80 {{value '-4096' out of range for constraint 'J'}}
+  // thumb2-error@80 {{value '-4096' out of range for constraint 'J'}}
+  __asm(
+      "movw %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(-4095), [ input ] "r"(i)
+      :);
+  // thumb1-error@89 {{value '-4095' out of range for constraint 'J'}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(-256), [ input ] "r"(i)
+      :);
+  // thumb1-error@95 {{value '-256' out of range for constraint 'J'}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(-255), [ input ] "r"(i)
+      :);
+  // No errors expected.
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(-1), [ input ] "r"(i)
+      :);
+  // No errors expected.
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(0), [ input ] "r"(i)
+      :);
+  // thumb1-error@113 {{value '0' out of range for constraint 'J'}}
+  __asm(
+      "movw %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(4095), [ input ] "r"(i)
+      :);
+  // thumb1-error@119 {{value '4095' out of range for constraint 'J'}}
+  __asm(
+      "movw %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "J"(4096), [ input ] "r"(i)
+      :);
+  // arm6-error@125 {{value '4096' out of range for constraint 'J'}}
+  // arm7-error@125 {{value '4096' out of range for constraint 'J'}}
+  // thumb1-error@125 {{value '4096' out of range for constraint 'J'}}
+  // thumb2-error@125 {{value '4096' out of range for constraint 'J'}}
+  return res;
+}
+
+// K: An immediate integer whose bitwise inverse is valid for a data-processing instruction. (ARM/Thumb2)
+//    An immediate integer between 0 and 255, with optional left-shift by some amount. (Thumb1)
+int test_K(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "K"(123), [ input ] "r"(i)
+      :);
+  // No errors expected.
+  return res;
+}
+
+// L: An immediate integer whose negation is valid for a data-processing instruction. (ARM/Thumb2)
+//    An immediate integer between -7 and 7. (Thumb1)
+int test_L(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "L"(-8), [ input ] "r"(i)
+      :); // thumb1-error@154 {{value '-8' out of range for constraint 'L'}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "L"(-7), [ input ] "r"(i)
+      :); // No errors expected.
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "L"(7), [ input ] "r"(i)
+      :); // No errors expected.
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "L"(8), [ input ] "r"(i)
+      :); // thumb1-error@169 {{value '8' out of range for constraint 'L'}}
+  return res;
+}
+
+// M: A power of two or a integer between 0 and 32. (ARM/Thumb2)
+//    An immediate integer which is a multiple of 4 between 0 and 1020. (Thumb1)
+int test_M(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "M"(123), [ input ] "r"(i)
+      :); // No errors expected.
+  return res;
+}
+
+// N: Invalid (ARM/Thumb2)
+//    An immediate integer between 0 and 31. (Thumb1)
+int test_N(int i) {
+  int res;
+  __asm("add %0, %1;"
+        : [ result ] "=r"(res)
+        : [ constant ] "N"(-1), [ input ] "r"(i)
+        :);
+  // arm6-error@192 {{invalid input constraint 'N' in asm}}
+  // arm7-error@192 {{invalid input constraint 'N' in asm}}
+  // thumb1-error@192 {{value '-1' out of range for constraint 'N'}}
+  // thumb2-error@192 {{invalid input constraint 'N' in asm}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "N"(0), [ input ] "r"(i)
+      :);
+  // arm6-error@201 {{invalid input constraint 'N' in asm}}
+  // arm7-error@201 {{invalid input constraint 'N' in asm}}
+  // thumb1-no-error
+  // thumb2-error@201 {{invalid input constraint 'N' in asm}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "N"(31), [ input ] "r"(i)
+      :);
+  // arm6-error@210 {{invalid input constraint 'N' in asm}}
+  // arm7-error@210 {{invalid input constraint 'N' in asm}}
+  // thumb1-no-error
+  // thumb2-error@210 {{invalid input constraint 'N' in asm}}
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "N"(32), [ input ] "r"(i)
+      :);
+  // arm6-error@219 {{invalid input constraint 'N' in asm}}
+  // arm7-error@219 {{invalid input constraint 'N' in asm}}
+  // thumb1-error@219 {{value '32' out of range for constraint 'N'}}
+  // thumb2-error@219 {{invalid input constraint 'N' in asm}}
+  return res;
+}
+
+// O: Invalid (ARM/Thumb2)
+//    An immediate integer which is a multiple of 4 between -508 and 508. (Thumb1)
+int test_O(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "O"(1), [ input ] "r"(i)
+      :);
+  // arm6-error@235 {{invalid input constraint 'O' in asm}}
+  // arm7-error@235 {{invalid input constraint 'O' in asm}}
+  // thumb1-no-error
+  // thumb2-error@235 {{invalid input constraint 'O' in asm}}
+  return res;
+}
+
+// l: Same as r (ARM)
+//    A low 32-bit GPR register (r0-r7). (Thumb1/Thumb2)
+int test_l(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=l"(res)
+      : [ constant ] "i"(10), [ input ] "l"(i)
+      :); // No errors expected.
+  return res;
+}
+
+// h: Invalid (ARM)
+//    A high 32-bit GPR register (r8-r15). (Thumb1/Thumb2)
+int test_h(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=h"(res)
+      : [ constant ] "i"(10), [ input ] "h"(i)
+      :);
+  // arm6-error@262 {{invalid output constraint '=h' in asm}}
+  // arm7-error@262 {{invalid output constraint '=h' in asm}}
+  return res;
+}
+
+// s: An integer constant, but allowing only relocatable values.
+int g;
+
+int test_s(int i) {
+  int res;
+  __asm(
+      "add %0, %1;"
+      : [ result ] "=r"(res)
+      : [ constant ] "s"(&g), [ input ] "r"(i)
+      :); // No errors expected.
+  return res;
+}
+
+// w: A 32, 64, or 128-bit floating-point/SIMD register: s0-s31, d0-d31, or q0-q15.
+float test_w(float x) {
+  __asm__("vsqrt.f32 %0, %1"
+          : "=w"(x)
+          : "w"(x)); // No error expected.
+  return x;
+}
+
+// x: A 32, 64, or 128-bit floating-point/SIMD register: s0-s15, d0-d7, or q0-q3.
+float test_x(float x) {
+  __asm__("vsqrt.f32 %0, %1"
+          : "=x"(x)
+          : "x"(x)); // No error expected.
+  return x;
+}
+
+// t: A 32, 64, or 128-bit floating-point/SIMD register: s0-s31, d0-d15, or q0-q7.
+float test_t(float x) {
+  __asm__("vsqrt.f32 %0, %1"
+          : "=t"(x)
+          : "t"(x)); // No error expected.
+  return x;
+}

llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

case 'M': case 'N': case 'O':
     // that the value fits in an int.
     if (CVal != CVal64)
       return;
 
     switch (ConstraintLetter) {
       case 'j':
         // Constant suitable for movw, must be between 0 and
         // 65535.
-        if (Subtarget->hasV6T2Ops())
+        if (Subtarget->hasV6T2Ops() || (Subtarget->hasV8MBaselineOps()))
           if (CVal >= 0 && CVal <= 65535)
             break;
         return;
       case 'I':
         if (Subtarget->isThumb1Only()) {
           // This must be a constant between 0 and 255, for ADD
           // immediates.
           if (CVal >= 0 && CVal <= 255)
switch (ConstraintLetter) {
           // GCC for the shift amount on shifted register operands, but it is
           // useful in general for any shift amounts.
           if ((CVal >= 0 && CVal <= 32) || ((CVal & (CVal - 1)) == 0))
             break;
         }
         return;
 
       case 'N':
-        if (Subtarget->isThumb()) {  // FIXME thumb2
+        if (Subtarget->isThumb1Only()) {
           // This must be a constant between 0 and 31, for shift amounts.
           if (CVal >= 0 && CVal <= 31)
             break;
         }
         return;
 
       case 'O':
-        if (Subtarget->isThumb()) {  // FIXME thumb2
+        if (Subtarget->isThumb1Only()) {
           // This must be a multiple of 4 between -508 and 508, for
           // ADD/SUB sp = sp + immediate.
           if ((CVal >= -508 && CVal <= 508) && ((CVal & 3) == 0))
             break;
         }
         return;
     }
     Result = DAG.getTargetConstant(CVal, SDLoc(Op), Op.getValueType());