Improve diagnostics for asm immediate constraints and downgrade from error to default-error

[joerg]

Summary

After r225244 clang verifies that inline asm constraints for immediate operands are used with ICE. This breaks existing code like pixman and tomcrypt as well as makes it impossible to create optimal output if either fixed or variable arguments can be supported. The backend has to be able to deal with the constraint check anyway and for all given cases it does. Therefore, downgrade the diagnostic from a hard error to an error-default warning. Correct use involving __builtin_constant_p can be annotated accordingly.

[rnk]

I'm in favor of the diagnostic change, I wasn't aware of how bad it was.

Does anyone else mind this being a default error warning? I'd prefer a hard error. I don't understand your argument about this diagnostic making it impossible to get "optimal output". It is simply a matter of using a macro instead of an always_inline function.

[compnerd]

I like the new diagnostic message. I think we should get that committed (and merged for the 3.6 release). However, Im not sure I agree with the downgrading of the error in the case of the failure to validate the constant though.

[joerg]

Using a macro requires code changes. That's more work when dealing with 3rd party code, so I do prefer non-intrusive changes.

[joerg]

Also, using functions gives better typing in terms of documentation, so it seems reasonable to continue to support that.
For optimality I meant things like:

unsigned char inb (unsigned short int __port)
{
  unsigned char _v;

  if (__builtin_constant_p(__port) && __port < 256)
    __asm__ __volatile__ ("inb %w1,%0":"=a" (_v):"N" (__port));
  else
    __asm__ __volatile__ ("inb %w1,%0":"=a" (_v):"d" (__port));
  return _v;
}

include/clang/Basic/DiagnosticGroups.td

   ]>;
 
 // Inline ASM warnings.
+def ASMImmediateOperand : DiagGroup<"asm-immediate-operand">;
 def ASMOperandWidths : DiagGroup<"asm-operand-widths">;
 def ASM : DiagGroup<"asm", [
-    ASMOperandWidths
+    ASMImmediateOperand, ASMOperandWidths
   ]>;
 
 // OpenMP warnings.

include/clang/Basic/DiagnosticSemaKinds.td

     "invalid lvalue in asm input for constraint '%0'">;
   def err_asm_invalid_input_constraint : Error<
     "invalid input constraint '%0' in asm">;
+  def warn_asm_immediate_expected : Warning<"constraint '%0' expects "
+    "an integer constant expression">, DefaultError,
+    InGroup<ASMImmediateOperand>;
   def err_asm_invalid_type_in_input : Error<
     "invalid type %0 in asm input for constraint '%1'">;
   def err_asm_tying_incompatible_types : Error<

lib/Sema/SemaStmtAsm.cpp

                          << InputExpr->getSourceRange());
     } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
       llvm::APSInt Result;
-      if (!InputExpr->EvaluateAsInt(Result, Context))
-        return StmtError(
-            Diag(InputExpr->getLocStart(), diag::err_asm_invalid_type_in_input)
-            << InputExpr->getType() << Info.getConstraintStr()
-            << InputExpr->getSourceRange());
-      if (Result.slt(Info.getImmConstantMin()) ||
-          Result.sgt(Info.getImmConstantMax()))
+      if (!InputExpr->EvaluateAsInt(Result, Context)) {
+        Diag(InputExpr->getLocStart(), diag::warn_asm_immediate_expected)
+          << Info.getConstraintStr() << InputExpr->getSourceRange();
+      } else if (Result.slt(Info.getImmConstantMin()) ||
+                 Result.sgt(Info.getImmConstantMax()))
         return StmtError(Diag(InputExpr->getLocStart(),
                               diag::err_invalid_asm_value_for_constraint)
                          << Result.toString(10) << Info.getConstraintStr()

test/Sema/inline-asm-validate-x86.c

   static const int AboveMax = 32;
   __asm__("xorl %0,%2"
           : "=r"(i)
-          : "0"(i), "I"(j)); // expected-error{{invalid type 'int' in asm input for constraint 'I'}}
+          : "0"(i), "I"(j)); // expected-error{{constraint 'I' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "I"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'I'}}

   static const int AboveMax = 64;
   __asm__("xorl %0,%2"
           : "=r"(i)
-          : "0"(i), "J"(j)); // expected-error{{invalid type 'int' in asm input for constraint 'J'}}
+          : "0"(i), "J"(j)); // expected-error{{constraint 'J' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "J"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'J'}}

   static const int AboveMax = 128;
   __asm__("xorl %0,%2"
           : "=r"(i)
-          : "0"(i), "K"(j)); // expected-error{{invalid type 'int' in asm input for constraint 'K'}}
+          : "0"(i), "K"(j)); // expected-error{{constraint 'K' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "K"(BelowMin)); // expected-error{{value '-129' out of range for constraint 'K'}}

   static const int AboveMax = 4;
   __asm__("xorl %0,%2"
           : "=r"(i)
-          : "0"(i), "M"(j)); // expected-error{{invalid type 'int' in asm input for constraint 'M'}}
+          : "0"(i), "M"(j)); // expected-error{{constraint 'M' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "M"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'M'}}

   static const int AboveMax = 256;
   __asm__("xorl %0,%2"
           : "=r"(i)
-          : "0"(i), "N"(j)); // expected-error{{invalid type 'int' in asm input for constraint 'N'}}
+          : "0"(i), "N"(j)); // expected-error{{constraint 'N' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "N"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'N'}}

   static const int AboveMax = 128;
   __asm__("xorl %0,%2"
           : "=r"(i)
-          : "0"(i), "O"(j)); // expected-error{{invalid type 'int' in asm input for constraint 'O'}}
+          : "0"(i), "O"(j)); // expected-error{{constraint 'O' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "O"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'O'}}