r216260 - [AArch64, inline-asm] Improve diagnostic that is printed when the size of a
Akira Hatanaka
ahatanaka at apple.com
Thu Aug 21 23:05:22 PDT 2014
Author: ahatanak
Date: Fri Aug 22 01:05:21 2014
New Revision: 216260
URL: http://llvm.org/viewvc/llvm-project?rev=216260&view=rev
Log:
[AArch64, inline-asm] Improve diagnostic that is printed when the size of a
variable that has regiser constraint "r" is not 64-bit.
General register operands are output using 64-bit "x" register names, regardless
of the size of the variable, unless the asm operand is prefixed with the "%w"
modifier. This surprises and confuses many users who aren't familiar with
aarch64 inline assembly rules.
With this commit, a note and fixit hint are printed which tell the users that
they need modifier "%w" in order to output a "w" register instead of an "x"
register.
<rdar://problem/12764785>
Added:
cfe/trunk/test/Sema/inline-asm-validate-aarch64.c
Modified:
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/AST/Stmt.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/Sema/arm64-inline-asm.c
cfe/trunk/test/Sema/inline-asm-validate.c
Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Fri Aug 22 01:05:21 2014
@@ -1580,18 +1580,21 @@ public:
Kind MyKind;
std::string Str;
unsigned OperandNo;
+
+ // Source range for operand references.
+ CharSourceRange Range;
public:
AsmStringPiece(const std::string &S) : MyKind(String), Str(S) {}
- AsmStringPiece(unsigned OpNo, char Modifier)
- : MyKind(Operand), Str(), OperandNo(OpNo) {
- Str += Modifier;
+ AsmStringPiece(unsigned OpNo, const std::string &S, SourceLocation Begin,
+ SourceLocation End)
+ : MyKind(Operand), Str(S), OperandNo(OpNo),
+ Range(CharSourceRange::getCharRange(Begin, End)) {
}
bool isString() const { return MyKind == String; }
bool isOperand() const { return MyKind == Operand; }
const std::string &getString() const {
- assert(isString());
return Str;
}
@@ -1600,12 +1603,14 @@ public:
return OperandNo;
}
+ CharSourceRange getRange() const {
+ assert(isOperand() && "Range is currently used only for Operands.");
+ return Range;
+ }
+
/// getModifier - Get the modifier for this operand, if present. This
/// returns '\0' if there was no modifier.
- char getModifier() const {
- assert(isOperand());
- return Str[0];
- }
+ char getModifier() const;
};
/// AnalyzeAsmString - Analyze the asm string of the current asm, decomposing
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug 22 01:05:21 2014
@@ -6038,6 +6038,9 @@ let CategoryName = "Inline Assembly Issu
"value size does not match register size specified by the constraint "
"and modifier">,
InGroup<ASMOperandWidths>;
+
+ def note_asm_missing_constraint_modifier : Note<
+ "use constraint modifier \"%0\"">;
}
let CategoryName = "Semantic Issue" in {
Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Fri Aug 22 01:05:21 2014
@@ -581,9 +581,11 @@ public:
unsigned /*Size*/) const {
return true;
}
- virtual bool validateConstraintModifier(StringRef /*Constraint*/,
- const char /*Modifier*/,
- unsigned /*Size*/) const {
+ virtual bool
+ validateConstraintModifier(StringRef /*Constraint*/,
+ char /*Modifier*/,
+ unsigned /*Size*/,
+ std::string &/*SuggestedModifier*/) const {
return true;
}
bool resolveSymbolicName(const char *&Name,
Modified: cfe/trunk/lib/AST/Stmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Stmt.cpp (original)
+++ cfe/trunk/lib/AST/Stmt.cpp Fri Aug 22 01:05:21 2014
@@ -357,6 +357,11 @@ unsigned AsmStmt::getNumPlusOperands() c
return Res;
}
+char GCCAsmStmt::AsmStringPiece::getModifier() const {
+ assert(isOperand() && "Only Operands can have modifiers.");
+ return isLetter(Str[0]) ? Str[0] : '\0';
+}
+
StringRef GCCAsmStmt::getClobber(unsigned i) const {
return getClobberStringLiteral(i)->getString();
}
@@ -517,17 +522,25 @@ unsigned GCCAsmStmt::AnalyzeAsmString(Sm
CurStringPiece.clear();
}
- // Handle %x4 and %x[foo] by capturing x as the modifier character.
- char Modifier = '\0';
+ // Handle operands that have asmSymbolicName (e.g., %x[foo]) and those that
+ // don't (e.g., %x4). 'x' following the '%' is the constraint modifier.
+
+ const char *Begin = CurPtr - 1; // Points to the character following '%'.
+ const char *Percent = Begin - 1; // Points to '%'.
+
if (isLetter(EscapedChar)) {
if (CurPtr == StrEnd) { // Premature end.
DiagOffs = CurPtr-StrStart-1;
return diag::err_asm_invalid_escape;
}
- Modifier = EscapedChar;
EscapedChar = *CurPtr++;
}
+ const TargetInfo &TI = C.getTargetInfo();
+ const SourceManager &SM = C.getSourceManager();
+ const LangOptions &LO = C.getLangOpts();
+
+ // Handle operands that don't have asmSymbolicName (e.g., %x4).
if (isDigit(EscapedChar)) {
// %n - Assembler operand n
unsigned N = 0;
@@ -543,11 +556,21 @@ unsigned GCCAsmStmt::AnalyzeAsmString(Sm
return diag::err_asm_invalid_operand_number;
}
- Pieces.push_back(AsmStringPiece(N, Modifier));
+ // Str contains "x4" (Operand without the leading %).
+ std::string Str(Begin, CurPtr - Begin);
+
+ // (BeginLoc, EndLoc) represents the range of the operand we are currently
+ // processing. Unlike Str, the range includes the leading '%'.
+ SourceLocation BeginLoc =
+ getAsmString()->getLocationOfByte(Percent - StrStart, SM, LO, TI);
+ SourceLocation EndLoc =
+ getAsmString()->getLocationOfByte(CurPtr - StrStart, SM, LO, TI);
+
+ Pieces.push_back(AsmStringPiece(N, Str, BeginLoc, EndLoc));
continue;
}
- // Handle %[foo], a symbolic operand reference.
+ // Handle operands that have asmSymbolicName (e.g., %x[foo]).
if (EscapedChar == '[') {
DiagOffs = CurPtr-StrStart-1;
@@ -566,7 +589,18 @@ unsigned GCCAsmStmt::AnalyzeAsmString(Sm
DiagOffs = CurPtr-StrStart;
return diag::err_asm_unknown_symbolic_operand_name;
}
- Pieces.push_back(AsmStringPiece(N, Modifier));
+
+ // Str contains "x[foo]" (Operand without the leading %).
+ std::string Str(Begin, NameEnd + 1 - Begin);
+
+ // (BeginLoc, EndLoc) represents the range of the operand we are currently
+ // processing. Unlike Str, the range includes the leading '%'.
+ SourceLocation BeginLoc =
+ getAsmString()->getLocationOfByte(Percent - StrStart, SM, LO, TI);
+ SourceLocation EndLoc =
+ getAsmString()->getLocationOfByte(NameEnd + 1 - StrStart, SM, LO, TI);
+
+ Pieces.push_back(AsmStringPiece(N, Str, BeginLoc, EndLoc));
CurPtr = NameEnd+1;
continue;
Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Fri Aug 22 01:05:21 2014
@@ -4145,8 +4145,10 @@ public:
}
return R;
}
- bool validateConstraintModifier(StringRef Constraint, const char Modifier,
- unsigned Size) const override {
+ bool
+ validateConstraintModifier(StringRef Constraint, const char Modifier,
+ unsigned Size,
+ std::string &SuggestedModifier) const override {
bool isOutput = (Constraint[0] == '=');
bool isInOut = (Constraint[0] == '+');
@@ -4592,9 +4594,10 @@ public:
return false;
}
- virtual bool validateConstraintModifier(StringRef Constraint,
- const char Modifier,
- unsigned Size) const {
+ bool
+ validateConstraintModifier(StringRef Constraint, const char Modifier,
+ unsigned Size,
+ std::string &SuggestedModifier) const override {
// Strip off constraint modifiers.
while (Constraint[0] == '=' || Constraint[0] == '+' || Constraint[0] == '&')
Constraint = Constraint.substr(1);
@@ -4613,7 +4616,11 @@ public:
default:
// By default an 'r' constraint will be in the 'x'
// registers.
- return Size == 64;
+ if (Size == 64)
+ return true;
+
+ SuggestedModifier = "w";
+ return false;
}
}
}
Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Fri Aug 22 01:05:21 2014
@@ -257,11 +257,22 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
continue;
unsigned Size = Context.getTypeSize(Ty);
- if (!Context.getTargetInfo()
- .validateConstraintModifier(Literal->getString(), Piece.getModifier(),
- Size))
+ std::string SuggestedModifier;
+ if (!Context.getTargetInfo().validateConstraintModifier(
+ Literal->getString(), Piece.getModifier(), Size,
+ SuggestedModifier)) {
Diag(Exprs[ConstraintIdx]->getLocStart(),
diag::warn_asm_mismatched_size_modifier);
+
+ if (!SuggestedModifier.empty()) {
+ auto B = Diag(Piece.getRange().getBegin(),
+ diag::note_asm_missing_constraint_modifier)
+ << SuggestedModifier;
+ SuggestedModifier = "%" + SuggestedModifier + Piece.getString();
+ B.AddFixItHint(FixItHint::CreateReplacement(Piece.getRange(),
+ SuggestedModifier));
+ }
+ }
}
// Validate tied input operands for type mismatches.
Modified: cfe/trunk/test/Sema/arm64-inline-asm.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/arm64-inline-asm.c?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/test/Sema/arm64-inline-asm.c (original)
+++ cfe/trunk/test/Sema/arm64-inline-asm.c Fri Aug 22 01:05:21 2014
@@ -5,5 +5,5 @@ void foo() {
asm volatile("USE(%x0)" :: "z"(0LL));
asm volatile("USE(%w0)" :: "z"(0));
- asm volatile("USE(%0)" :: "z"(0)); // expected-warning {{value size does not match register size specified by the constraint and modifier}}
+ asm volatile("USE(%0)" :: "z"(0)); // expected-warning {{value size does not match register size specified by the constraint and modifier}} expected-note {{use constraint modifier "w"}}
}
Added: cfe/trunk/test/Sema/inline-asm-validate-aarch64.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline-asm-validate-aarch64.c?rev=216260&view=auto
==============================================================================
--- cfe/trunk/test/Sema/inline-asm-validate-aarch64.c (added)
+++ cfe/trunk/test/Sema/inline-asm-validate-aarch64.c Fri Aug 22 01:05:21 2014
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple arm64-apple-darwin -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+typedef unsigned char uint8_t;
+
+uint8_t constraint_r(uint8_t *addr) {
+ uint8_t byte;
+
+ __asm__ volatile("ldrb %0, [%1]" : "=r" (byte) : "r" (addr) : "memory");
+// CHECK: warning: value size does not match register size specified by the constraint and modifier
+// CHECK: note: use constraint modifier "w"
+// CHECK: fix-it:{{.*}}:{8:26-8:28}:"%w0"
+
+ return byte;
+}
+
+uint8_t constraint_r_symbolic(uint8_t *addr) {
+ uint8_t byte;
+
+ __asm__ volatile("ldrb %[s0], [%[s1]]" : [s0] "=r" (byte) : [s1] "r" (addr) : "memory");
+// CHECK: warning: value size does not match register size specified by the constraint and modifier
+// CHECK: note: use constraint modifier "w"
+// CHECK: fix-it:{{.*}}:{19:26-19:31}:"%w[s0]"
+
+ return byte;
+}
+
+#define PERCENT "%"
+
+uint8_t constraint_r_symbolic_macro(uint8_t *addr) {
+ uint8_t byte;
+
+ __asm__ volatile("ldrb "PERCENT"[s0], [%[s1]]" : [s0] "=r" (byte) : [s1] "r" (addr) : "memory");
+// CHECK: warning: value size does not match register size specified by the constraint and modifier
+// CHECK: note: use constraint modifier "w"
+// CHECK-NOT: fix-it
+
+ return byte;
+}
Modified: cfe/trunk/test/Sema/inline-asm-validate.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline-asm-validate.c?rev=216260&r1=216259&r2=216260&view=diff
==============================================================================
--- cfe/trunk/test/Sema/inline-asm-validate.c (original)
+++ cfe/trunk/test/Sema/inline-asm-validate.c Fri Aug 22 01:05:21 2014
@@ -3,6 +3,6 @@
unsigned t, r, *p;
int foo (void) {
- __asm__ __volatile__( "stxr %w[_t], %[_r], [%[_p]]" : [_t] "=&r" (t) : [_p] "p" (p), [_r] "r" (r) : "memory"); // expected-warning{{value size does not match register size specified by the constraint and modifier}}
+ __asm__ __volatile__( "stxr %w[_t], %[_r], [%[_p]]" : [_t] "=&r" (t) : [_p] "p" (p), [_r] "r" (r) : "memory"); // expected-warning{{value size does not match register size specified by the constraint and modifier}} expected-note {{use constraint modifier "w"}}
return 1;
}
More information about the cfe-commits
mailing list