diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 3e91d74cd66a9..a4e1975469566 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1755,8 +1755,10 @@ def ObjCStrictPotentiallyDirectSelector : // Inline ASM warnings. def ASMOperandWidths : DiagGroup<"asm-operand-widths">; +def ASMUnusedOperand : DiagGroup<"unused-asm-operand">; def ASM : DiagGroup<"asm", [ - ASMOperandWidths + ASMOperandWidths, + ASMUnusedOperand ]>; // Linker warnings. diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6c4339b6175eb..7b35a1eeceae3 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10144,6 +10144,10 @@ let CategoryName = "Inline Assembly Issue" in { "value size does not match register size specified by the constraint " "and modifier">, InGroup; + def warn_unused_asm_operand : Warning< + "unused asm %enum_select{%Output{output}|%Input{input}}0" + " operand">, + InGroup, DefaultIgnore; def note_asm_missing_constraint_modifier : Note< "use constraint modifier \"%0\"">; diff --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp index 6bf12d9cd98da..61da7a0b0cbb8 100644 --- a/clang/lib/Sema/SemaStmtAsm.cpp +++ b/clang/lib/Sema/SemaStmtAsm.cpp @@ -21,6 +21,7 @@ #include "clang/Sema/Scope.h" #include "clang/Sema/ScopeInfo.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" @@ -103,24 +104,6 @@ static bool CheckAsmLValue(Expr *E, Sema &S) { return true; } -/// isOperandMentioned - Return true if the specified operand # is mentioned -/// anywhere in the decomposed asm string. -static bool -isOperandMentioned(unsigned OpNo, - ArrayRef AsmStrPieces) { - for (unsigned p = 0, e = AsmStrPieces.size(); p != e; ++p) { - const GCCAsmStmt::AsmStringPiece &Piece = AsmStrPieces[p]; - if (!Piece.isOperand()) - continue; - - // If this is a reference to the input and if the input was the smaller - // one, then we have to reject this asm. - if (Piece.getOperandNo() == OpNo) - return true; - } - return false; -} - static bool CheckNakedParmReference(Expr *E, Sema &S) { FunctionDecl *Func = dyn_cast(S.CurContext); if (!Func) @@ -560,7 +543,11 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, return NS; } - // Validate constraints and modifiers. + // Validate constraints and modifiers, and (cheaply, piggybacking on this + // same walk over Pieces rather than doing a second pass over the asm + // string) track which operands the template string actually references, + // so unreferenced ones can be flagged below. + llvm::SmallBitVector UsedOperands(NumOutputs + NumInputs); for (unsigned i = 0, e = Pieces.size(); i != e; ++i) { GCCAsmStmt::AsmStringPiece &Piece = Pieces[i]; if (!Piece.isOperand()) continue; @@ -586,6 +573,8 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, " AnalyzeAsmString"); } + UsedOperands.set(ConstraintIdx); + // Now that we have the right indexes go ahead and check. Expr *Constraint = constraints[ConstraintIdx]; const Type *Ty = Exprs[ConstraintIdx]->getType().getTypePtr(); @@ -613,6 +602,25 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, } } + // Warn about output/input operands the asm template string never + // references. Skip tied inputs (matched by a numeric constraint, e.g. + // "0"): by convention only the output's own number is used to reference + // that storage, so the tied input's own index is never expected to be + // referenced on its own. + if (!UsedOperands.all()) { + for (unsigned i = 0; i != NumOutputs; ++i) + if (!UsedOperands[i]) + targetDiag(Exprs[i]->getBeginLoc(), diag::warn_unused_asm_operand) + << diag::AsmOperandKind::Output; + + for (unsigned i = 0; i != NumInputs; ++i) + if (!UsedOperands[NumOutputs + i] && + !InputConstraintInfos[i].hasTiedOperand()) + targetDiag(Exprs[NumOutputs + i]->getBeginLoc(), + diag::warn_unused_asm_operand) + << diag::AsmOperandKind::Input; + } + // Validate tied input operands for type mismatches. unsigned NumAlternatives = ~0U; for (unsigned i = 0, e = OutputConstraintInfos.size(); i != e; ++i) { @@ -713,13 +721,13 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, // If this is a reference to the input and if the input was the smaller // one, then we have to reject this asm. - if (isOperandMentioned(InputOpNo, Pieces)) { + if (UsedOperands[InputOpNo]) { // This is a use in the asm string of the smaller operand. Since we // codegen this by promoting to a wider value, the asm will get printed // "wrong". SmallerValueMentioned |= InSize < OutSize; } - if (isOperandMentioned(TiedTo, Pieces)) { + if (UsedOperands[TiedTo]) { // If this is a reference to the output, and if the output is the larger // value, then it's ok because we'll promote the input to the larger type. SmallerValueMentioned |= OutSize < InSize; @@ -754,8 +762,7 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, // integer, unmentioned, and is a constant, then we'll allow truncating it // down to the size of the destination. if (InputDomain == AD_Int && OutputDomain == AD_Int && - !isOperandMentioned(InputOpNo, Pieces) && - InputExpr->isEvaluatable(Context)) { + !UsedOperands[InputOpNo] && InputExpr->isEvaluatable(Context)) { CastKind castKind = (OutTy->isBooleanType() ? CK_IntegralToBoolean : CK_IntegralCast); InputExpr = ImpCastExprToType(InputExpr, OutTy, castKind).get(); diff --git a/clang/test/Sema/warn-unused-asm-operand.c b/clang/test/Sema/warn-unused-asm-operand.c new file mode 100644 index 0000000000000..932146c150a21 --- /dev/null +++ b/clang/test/Sema/warn-unused-asm-operand.c @@ -0,0 +1,57 @@ +// RUN: %clang_cc1 -fsyntax-only -verify=silent %s +// RUN: %clang_cc1 -fsyntax-only -Wunused-asm-operand -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wasm -verify %s + +// silent-no-diagnostics + +int add(int a, int b) { + int r; + // All operands are referenced: no warning. + asm("add %1, %2, %0" : "=r"(r) : "r"(a), "r"(b)); + return r; +} + +int named(int x) { + int r; + // Named operands are still tracked correctly: no warning. + asm("mov %[in], %[out]" : [out] "=r"(r) : [in] "r"(x)); + return r; +} + +int tied(int a) { + int r; + // A numerically-tied input ("0") is referenced via the output's own + // number by convention, not its own: no warning for the input. + asm("inc %0" : "=r"(r) : "0"(a)); + return r; +} + +void readwrite(int *p) { + // A read-write operand is a single slot referenced via %0: no warning. + asm("incl %0" : "+r"(*p)); +} + +int unused_output(int a) { + int r; + // Neither operand is referenced by the template: both warn. + // expected-warning@+2 {{unused asm output operand}} + // expected-warning@+1 {{unused asm input operand}} + asm("nop" : "=r"(r) : "r"(a)); + return a; +} + +int unused_input(int a, int b) { + int r; + // expected-warning@+1 {{unused asm input operand}} + asm("add %1, %1, %0" : "=r"(r) : "r"(a), "r"(b)); + return r; +} + +int unused_alongside_tied(int a, int b) { + int r; + // The tied operand ("0") stays silent; only the genuinely-unreferenced + // one warns. + // expected-warning@+1 {{unused asm input operand}} + asm("inc %0" : "=r"(r) : "0"(a), "r"(b)); + return r; +}