[clang] Improve diagnostics for constraints of inline asm (NFC)

clang/include/clang/Basic/DiagnosticCommonKinds.td

@@ -309,6 +309,39 @@ def err_asm_invalid_type : Error<
 def err_ms_asm_bitfield_unsupported : Error<
   "an inline asm block cannot have an operand which is a bit-field">;
 
+def asm_invalid_constraint_generic : TextSubstitution<
+  "invalid %select{input|output}0 constraint '%1' in asm">;
+def err_asm_invalid_constraint : Error<
+  "%sub{asm_invalid_constraint_generic}0,1">;
+def err_asm_invalid_constraint_start : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: output constraint must start with"
+  " '=' or '+'">;
+def err_asm_invalid_constraint_rw_clobber : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: early clobber with a read-write"
+  " constraint must be a register">;
+def err_asm_invalid_constraint_mem_or_reg : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: constraint must allow either"
+  " memory or register operands">;
+def err_asm_invalid_constraint_missing_bracket : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: missing ']'">;
+def err_asm_invalid_constraint_wrong_symbol : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: no matching output constraint"
+  " with the specified name">;
+def err_asm_invalid_constraint_empty : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: empty constraint has been"
+  " provided">;
+def err_asm_invalid_constraint_oob : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: the index is out of bounds">;
+def err_asm_invalid_constraint_missing : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: references non-existent output"
+  " constraint">;
+def err_asm_invalid_constraint_wrongly_tied : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: tied constraint must be tied to"
+  " the same operand referenced to by the number">;
+def err_asm_invalid_constraint_output_only : Error<
+  "%sub{asm_invalid_constraint_generic}0,1: must refer to an output-only"
+  " operand">;
+
 def warn_stack_clash_protection_inline_asm : Warning<
   "unable to protect inline asm that clobbers stack pointer against stack "
   "clash">, InGroup<DiagGroup<"stack-protector">>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -9280,12 +9280,8 @@ let CategoryName = "Inline Assembly Issue" in {
     : Error<"cannot pass a pointer-to-member through register-constrained "
             "inline assembly parameter">;
   def err_asm_invalid_lvalue_in_output : Error<"invalid lvalue in asm output">;
-  def err_asm_invalid_output_constraint : Error<
-    "invalid output constraint '%0' in asm">;
   def err_asm_invalid_lvalue_in_input : Error<
     "invalid lvalue in asm input for constraint '%0'">;
-  def err_asm_invalid_input_constraint : Error<
-    "invalid input constraint '%0' in asm">;
   def err_asm_tying_incompatible_types : Error<
     "unsupported inline asm: input with type "
     "%diff{$ matching output with type $|}0,1">;

clang/include/clang/Basic/TargetInfo.h

@@ -17,6 +17,7 @@
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/BitmaskEnum.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Specifiers.h"
@@ -1196,10 +1197,12 @@ class TargetInfo : public TransferrableTargetInfo,
 
   // validateOutputConstraint, validateInputConstraint - Checks that
   // a constraint is valid and provides information about it.
-  // FIXME: These should return a real error instead of just true/false.
-  bool validateOutputConstraint(ConstraintInfo &Info) const;
-  bool validateInputConstraint(MutableArrayRef<ConstraintInfo> OutputConstraints,
-                               ConstraintInfo &info) const;
+  bool validateOutputConstraint(ConstraintInfo &Info,
+                                llvm::StringMap<bool> *FeatureMap,
+                                diag::kind &Diag) const;
+  bool validateInputConstraint(
+      MutableArrayRef<ConstraintInfo> OutputConstraints, ConstraintInfo &Info,
+      llvm::StringMap<bool> *FeatureMap, diag::kind &Diag) const;
 
   virtual bool validateOutputSize(const llvm::StringMap<bool> &FeatureMap,
                                   StringRef /*Constraint*/,
@@ -1219,13 +1222,14 @@ class TargetInfo : public TransferrableTargetInfo,
                              std::string &/*SuggestedModifier*/) const {
     return true;
   }
-  virtual bool
-  validateAsmConstraint(const char *&Name,
-                        TargetInfo::ConstraintInfo &info) const = 0;
+  virtual bool validateAsmConstraint(const char *&Name,
+                                     TargetInfo::ConstraintInfo &Info,
+                                     llvm::StringMap<bool> *FeatureMap,
+                                     diag::kind &Diag) const = 0;
 
   bool resolveSymbolicName(const char *&Name,
                            ArrayRef<ConstraintInfo> OutputConstraints,
-                           unsigned &Index) const;
+                           unsigned &Index, diag::kind &Diag) const;
 
   // Constraint parm will be left pointing at the last character of
   // the constraint.  In practice, it won't be changed unless the

clang/lib/Basic/TargetInfo.cpp

@@ -722,11 +722,15 @@ StringRef TargetInfo::getNormalizedGCCRegisterName(StringRef Name,
   return Name;
 }
 
-bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info) const {
+bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info,
+                                          llvm::StringMap<bool> *FeatureMap,
+                                          diag::kind &Diag) const {
   const char *Name = Info.getConstraintStr().c_str();
   // An output constraint must start with '=' or '+'
-  if (*Name != '=' && *Name != '+')
+  if (*Name != '=' && *Name != '+') {
+    Diag = diag::err_asm_invalid_constraint_start;
     return false;
+  }
 
   if (*Name == '+')
     Info.setIsReadWrite();
@@ -735,7 +739,7 @@ bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info) const {
   while (*Name) {
     switch (*Name) {
     default:
-      if (!validateAsmConstraint(Name, Info)) {
+      if (!validateAsmConstraint(Name, Info, FeatureMap, Diag)) {
         // FIXME: We temporarily return false
         // so we can add more constraints as we hit it.
         // Eventually, an unknown constraint should just be treated as 'g'.
@@ -788,17 +792,23 @@ bool TargetInfo::validateOutputConstraint(ConstraintInfo &Info) const {
 
   // Early clobber with a read-write constraint which doesn't permit registers
   // is invalid.
-  if (Info.earlyClobber() && Info.isReadWrite() && !Info.allowsRegister())
+  if (Info.earlyClobber() && Info.isReadWrite() && !Info.allowsRegister()) {
+    Diag = diag::err_asm_invalid_constraint_rw_clobber;
     return false;
+  }
 
   // If a constraint allows neither memory nor register operands it contains
   // only modifiers. Reject it.
-  return Info.allowsMemory() || Info.allowsRegister();
+  if (!Info.allowsMemory() && !Info.allowsRegister()) {
+    Diag = diag::err_asm_invalid_constraint_mem_or_reg;
+    return false;
+  }
+  return true;
 }
 
 bool TargetInfo::resolveSymbolicName(const char *&Name,
                                      ArrayRef<ConstraintInfo> OutputConstraints,
-                                     unsigned &Index) const {
+                                     unsigned &Index, diag::kind &Diag) const {
   assert(*Name == '[' && "Symbolic name did not start with '['");
   Name++;
   const char *Start = Name;
@@ -807,6 +817,7 @@ bool TargetInfo::resolveSymbolicName(const char *&Name,
 
   if (!*Name) {
     // Missing ']'
+    Diag = diag::err_asm_invalid_constraint_missing_bracket;
     return false;
   }
 
@@ -816,16 +827,19 @@ bool TargetInfo::resolveSymbolicName(const char *&Name,
     if (SymbolicName == OutputConstraints[Index].getName())
       return true;
 
+  Diag = diag::err_asm_invalid_constraint_wrong_symbol;
   return false;
 }
 
 bool TargetInfo::validateInputConstraint(
-                              MutableArrayRef<ConstraintInfo> OutputConstraints,
-                              ConstraintInfo &Info) const {
+    MutableArrayRef<ConstraintInfo> OutputConstraints, ConstraintInfo &Info,
+    llvm::StringMap<bool> *FeatureMap, diag::kind &Diag) const {
   const char *Name = Info.ConstraintStr.c_str();
 
-  if (!*Name)
+  if (!*Name) {
+    Diag = diag::err_asm_invalid_constraint_empty;
     return false;
+  }
 
   while (*Name) {
     switch (*Name) {
@@ -838,25 +852,34 @@ bool TargetInfo::validateInputConstraint(
         const char *DigitEnd = Name;
         unsigned i;
         if (StringRef(DigitStart, DigitEnd - DigitStart + 1)
-                .getAsInteger(10, i))
+                .getAsInteger(10, i)) {
+          Diag = diag::err_asm_invalid_constraint_oob;
           return false;
+        }
 
         // Check if matching constraint is out of bounds.
-        if (i >= OutputConstraints.size()) return false;
+        if (i >= OutputConstraints.size()) {
+          Diag = diag::err_asm_invalid_constraint_missing;
+          return false;
+        }
 
         // A number must refer to an output only operand.
-        if (OutputConstraints[i].isReadWrite())
+        if (OutputConstraints[i].isReadWrite()) {
+          Diag = diag::err_asm_invalid_constraint_output_only;
           return false;
+        }
 
         // If the constraint is already tied, it must be tied to the
         // same operand referenced to by the number.
-        if (Info.hasTiedOperand() && Info.getTiedOperand() != i)
+        if (Info.hasTiedOperand() && Info.getTiedOperand() != i) {
+          Diag = diag::err_asm_invalid_constraint_wrongly_tied;
           return false;
+        }
 
         // The constraint should have the same info as the respective
         // output constraint.
         Info.setTiedOperand(i, OutputConstraints[i]);
-      } else if (!validateAsmConstraint(Name, Info)) {
+      } else if (!validateAsmConstraint(Name, Info, FeatureMap, Diag)) {
         // FIXME: This error return is in place temporarily so we can
         // add more constraints as we hit it.  Eventually, an unknown
         // constraint should just be treated as 'g'.
@@ -865,17 +888,21 @@ bool TargetInfo::validateInputConstraint(
       break;
     case '[': {
       unsigned Index = 0;
-      if (!resolveSymbolicName(Name, OutputConstraints, Index))
+      if (!resolveSymbolicName(Name, OutputConstraints, Index, Diag))
         return false;
 
       // If the constraint is already tied, it must be tied to the
       // same operand referenced to by the number.
-      if (Info.hasTiedOperand() && Info.getTiedOperand() != Index)
+      if (Info.hasTiedOperand() && Info.getTiedOperand() != Index) {
+        Diag = diag::err_asm_invalid_constraint_wrongly_tied;
         return false;
+      }
 
       // A number must refer to an output only operand.
-      if (OutputConstraints[Index].isReadWrite())
+      if (OutputConstraints[Index].isReadWrite()) {
+        Diag = diag::err_asm_invalid_constraint_output_only;
         return false;
+      }
 
       Info.setTiedOperand(Index, OutputConstraints[Index]);
       break;
@@ -896,7 +923,7 @@ bool TargetInfo::validateInputConstraint(
     case 'N':
     case 'O':
     case 'P':
-      if (!validateAsmConstraint(Name, Info))
+      if (!validateAsmConstraint(Name, Info, FeatureMap, Diag))
         return false;
       break;
     case 'r': // general register.

clang/lib/Basic/Targets/AArch64.cpp

@@ -1339,8 +1339,10 @@ AArch64TargetInfo::convertConstraint(const char *&Constraint) const {
   return R;
 }
 
-bool AArch64TargetInfo::validateAsmConstraint(
-    const char *&Name, TargetInfo::ConstraintInfo &Info) const {
+bool AArch64TargetInfo::validateAsmConstraint(const char *&Name,
+                                              TargetInfo::ConstraintInfo &Info,
+                                              llvm::StringMap<bool> *FeatureMap,
+                                              diag::kind &Diag) const {
   switch (*Name) {
   default:
     return false;

clang/lib/Basic/Targets/AArch64.h

@@ -176,7 +176,9 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
   std::string convertConstraint(const char *&Constraint) const override;
 
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &Info) const override;
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override;
   bool
   validateConstraintModifier(StringRef Constraint, char Modifier, unsigned Size,
                              std::string &SuggestedModifier) const override;

clang/lib/Basic/Targets/AMDGPU.h

@@ -137,7 +137,9 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
   /// {s[n:m]}
   /// {a[n:m]}
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &Info) const override {
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override {
     static const ::llvm::StringSet<> SpecialRegs({
         "exec", "vcc", "flat_scratch", "m0", "scc", "tba", "tma",
         "flat_scratch_lo", "flat_scratch_hi", "vcc_lo", "vcc_hi", "exec_lo",
@@ -232,7 +234,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
 
     const char *Begin = Constraint;
     TargetInfo::ConstraintInfo Info("", "");
-    if (validateAsmConstraint(Constraint, Info))
+    diag::kind AsmDiag;
+    if (validateAsmConstraint(Constraint, Info, nullptr, AsmDiag))
       return std::string(Begin).substr(0, Constraint - Begin + 1);
 
     Constraint = Begin;

clang/lib/Basic/Targets/ARC.h

@@ -64,7 +64,9 @@ class LLVM_LIBRARY_VISIBILITY ARCTargetInfo : public TargetInfo {
   }
 
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &Info) const override {
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override {
     return false;
   }
 

clang/lib/Basic/Targets/ARM.cpp

@@ -1136,8 +1136,10 @@ ArrayRef<TargetInfo::GCCRegAlias> ARMTargetInfo::getGCCRegAliases() const {
   return llvm::ArrayRef(GCCRegAliases);
 }
 
-bool ARMTargetInfo::validateAsmConstraint(
-    const char *&Name, TargetInfo::ConstraintInfo &Info) const {
+bool ARMTargetInfo::validateAsmConstraint(const char *&Name,
+                                          TargetInfo::ConstraintInfo &Info,
+                                          llvm::StringMap<bool> *FeatureMap,
+                                          diag::kind &Diag) const {
   switch (*Name) {
   default:
     break;

clang/lib/Basic/Targets/ARM.h

@@ -204,7 +204,9 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
   ArrayRef<const char *> getGCCRegNames() const override;
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override;
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &Info) const override;
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override;
   std::string convertConstraint(const char *&Constraint) const override;
   bool
   validateConstraintModifier(StringRef Constraint, char Modifier, unsigned Size,

clang/lib/Basic/Targets/AVR.h

@@ -94,7 +94,9 @@ class LLVM_LIBRARY_VISIBILITY AVRTargetInfo : public TargetInfo {
   }
 
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &Info) const override {
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override {
     // There aren't any multi-character AVR specific constraints.
     if (StringRef(Name).size() > 1)
       return false;

clang/lib/Basic/Targets/BPF.h

@@ -72,7 +72,9 @@ class LLVM_LIBRARY_VISIBILITY BPFTargetInfo : public TargetInfo {
   }
 
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &Info) const override {
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override {
     switch (*Name) {
     default:
       break;

clang/lib/Basic/Targets/CSKY.cpp

@@ -289,8 +289,10 @@ ArrayRef<TargetInfo::GCCRegAlias> CSKYTargetInfo::getGCCRegAliases() const {
   return llvm::ArrayRef(GCCRegAliases);
 }
 
-bool CSKYTargetInfo::validateAsmConstraint(
-    const char *&Name, TargetInfo::ConstraintInfo &Info) const {
+bool CSKYTargetInfo::validateAsmConstraint(const char *&Name,
+                                           TargetInfo::ConstraintInfo &Info,
+                                           llvm::StringMap<bool> *FeatureMap,
+                                           diag::kind &Diag) const {
   switch (*Name) {
   default:
     return false;

clang/lib/Basic/Targets/CSKY.h

@@ -80,7 +80,9 @@ class LLVM_LIBRARY_VISIBILITY CSKYTargetInfo : public TargetInfo {
   }
 
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &info) const override;
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override;
 
   std::string_view getClobbers() const override { return ""; }
 

clang/lib/Basic/Targets/DirectX.h

@@ -83,7 +83,9 @@ class LLVM_LIBRARY_VISIBILITY DirectXTargetInfo : public TargetInfo {
   }
 
   bool validateAsmConstraint(const char *&Name,
-                             TargetInfo::ConstraintInfo &info) const override {
+                             TargetInfo::ConstraintInfo &Info,
+                             llvm::StringMap<bool> *FeatureMap,
+                             diag::kind &Diag) const override {
     return true;
   }