Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar added a comment. Hello :) I will prepare another patch as you suggested, is this patch good? Can it be submitted? Ziv https://reviews.llvm.org/D15075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24958: Test linked to D24957
zizhar created this revision. zizhar added a reviewer: rnk. zizhar added a subscriber: cfe-commits. zizhar set the repository for this revision to rL LLVM. A test for the patch in https://reviews.llvm.org/D24957 Repository: rL LLVM https://reviews.llvm.org/D24958 Files: test/CodeGen/ms-inline-asm.c Index: test/CodeGen/ms-inline-asm.c === --- test/CodeGen/ms-inline-asm.c +++ test/CodeGen/ms-inline-asm.c @@ -634,6 +634,15 @@ // CHECK: call void asm sideeffect inteldialect "jmp {{.*}}__MSASMLABEL_.5__dollar_label$$\0A\09{{.*}}__MSASMLABEL_.5__dollar_label$$:", "~{dirflag},~{fpsr},~{flags}"() } +void label6(){ + __asm { + jmp short label +label: + } + // CHECK-LABEL: define void @label6 + // CHECK: call void asm sideeffect inteldialect "jmp {{.*}}__MSASMLABEL_.6__label\0A\09{{.*}}__MSASMLABEL_.6__label:", "~{dirflag},~{fpsr},~{flags}"() +} + typedef union _LARGE_INTEGER { struct { unsigned int LowPart; Index: test/CodeGen/ms-inline-asm.c === --- test/CodeGen/ms-inline-asm.c +++ test/CodeGen/ms-inline-asm.c @@ -634,6 +634,15 @@ // CHECK: call void asm sideeffect inteldialect "jmp {{.*}}__MSASMLABEL_.5__dollar_label$$\0A\09{{.*}}__MSASMLABEL_.5__dollar_label$$:", "~{dirflag},~{fpsr},~{flags}"() } +void label6(){ + __asm { + jmp short label +label: + } + // CHECK-LABEL: define void @label6 + // CHECK: call void asm sideeffect inteldialect "jmp {{.*}}__MSASMLABEL_.6__label\0A\09{{.*}}__MSASMLABEL_.6__label:", "~{dirflag},~{fpsr},~{flags}"() +} + typedef union _LARGE_INTEGER { struct { unsigned int LowPart; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar updated this revision to Diff 73617. zizhar added a comment. Changed the '^' to point to the clobber which conflicts. Changed the relevant comments. However, regarding the point about the non-alphabetical characters - It's a different behavior for these specific characters, since these should be ignored and the default case is for an unknown character. https://reviews.llvm.org/D15075 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Basic/Targets.cpp lib/Headers/intrin.h lib/Sema/SemaStmtAsm.cpp test/Sema/asm.c Index: test/Sema/asm.c === --- test/Sema/asm.c +++ test/Sema/asm.c @@ -28,6 +28,16 @@ asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010 Index: lib/Sema/SemaStmtAsm.cpp === --- lib/Sema/SemaStmtAsm.cpp +++ lib/Sema/SemaStmtAsm.cpp @@ -22,6 +22,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -137,6 +138,58 @@ return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +StringRef ExtractRegisterName(const Expr *Expression, const TargetInfo &Target) { + while (const ImplicitCastExpr *P = dyn_cast(Expression)) { +Expression = P->getSubExpr(); + } + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { +// Handle cases where the expression is a variable +const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); +if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) +return Target.isValidGCCRegisterName(Attr->getLabel()) +? Target.getNormalizedGCCRegisterName(Attr->getLabel(), true) +: ""; +} + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list. If there's a conflict, returns the location of the +// conflicted clobber, else returns nullptr +SourceLocation* GetClobberConflictLocation(MultiExprArg Exprs, + StringLiteral **Constraints, StringLiteral **Clobbers, int NumClobbers, + const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { +StringRef Constraint = Constraints[i]->getString(); +StringRef InOutReg = Target.getConstraintRegister( + Constraint, ExtractRegisterName(Exprs[i], Target)); +if (InOutReg != "") + InOutVars.insert(InOutReg); + } + // Check for each item in the clobber list if it conflicts with the input + // or output + for (int i = 0; i < NumClobbers; ++i) { +StringRef Clobber = Clobbers[i]->getString(); +// We only check registers, therefore we don't check cc and memory clobbers +if (Clobber == "cc" || Clobber == "memory") + continue; +Clobber = Target.getNormalizedGCCRegisterName(Clobber, true); +// Go over the output's registers we collected +if (InOutVars.find(Clobber) != InOutVars.end()){ + SourceLocation Loc = Clobbers[i]->getLocStart(); + return &Loc; +} + } + return nullptr; +} + StmtResu
[PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar updated this revision to Diff 73760. zizhar added a comment. fixed :) https://reviews.llvm.org/D15075 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Basic/Targets.cpp lib/Headers/intrin.h lib/Sema/SemaStmtAsm.cpp test/Sema/asm.c Index: test/Sema/asm.c === --- test/Sema/asm.c +++ test/Sema/asm.c @@ -28,6 +28,16 @@ asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010 Index: lib/Sema/SemaStmtAsm.cpp === --- lib/Sema/SemaStmtAsm.cpp +++ lib/Sema/SemaStmtAsm.cpp @@ -22,6 +22,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -137,6 +138,56 @@ return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +StringRef ExtractRegisterName(const Expr *Expression, + const TargetInfo &Target) { + Expression = Expression->IgnoreImpCasts(); + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { +// Handle cases where the expression is a variable +const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); +if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) +return Target.isValidGCCRegisterName(Attr->getLabel()) +? Target.getNormalizedGCCRegisterName(Attr->getLabel(), true) +: ""; +} + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list. If there's a conflict, returns the location of the +// conflicted clobber, else returns nullptr +SourceLocation +GetClobberConflictLocation(MultiExprArg Exprs, StringLiteral **Constraints, +StringLiteral **Clobbers, int NumClobbers, +const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { +StringRef Constraint = Constraints[i]->getString(); +StringRef InOutReg = Target.getConstraintRegister( + Constraint, ExtractRegisterName(Exprs[i], Target)); +if (InOutReg != "") + InOutVars.insert(InOutReg); + } + // Check for each item in the clobber list if it conflicts with the input + // or output + for (int i = 0; i < NumClobbers; ++i) { +StringRef Clobber = Clobbers[i]->getString(); +// We only check registers, therefore we don't check cc and memory clobbers +if (Clobber == "cc" || Clobber == "memory") + continue; +Clobber = Target.getNormalizedGCCRegisterName(Clobber, true); +// Go over the output's registers we collected +if (InOutVars.count(Clobber)) + return Clobbers[i]->getLocStart(); + } + return SourceLocation(); +} + StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, bool IsVolatile, unsigned NumOutputs, unsigned NumInputs, IdentifierInfo **Names, @@ -542,7 +593,14 @@ << InputExpr->getSourceRange(); return StmtError(); } - + + // Check for conflicts between clobber list and input or output lists + SourceLocati
[PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar updated this revision to Diff 74068. zizhar added a comment. I hope this last update finishes it, Thanks for the review :) https://reviews.llvm.org/D15075 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Basic/Targets.cpp lib/Headers/intrin.h lib/Sema/SemaStmtAsm.cpp test/Sema/asm.c Index: test/Sema/asm.c === --- test/Sema/asm.c +++ test/Sema/asm.c @@ -28,6 +28,16 @@ asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010 Index: lib/Sema/SemaStmtAsm.cpp === --- lib/Sema/SemaStmtAsm.cpp +++ lib/Sema/SemaStmtAsm.cpp @@ -22,6 +22,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -137,6 +138,57 @@ return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +StringRef ExtractRegisterName(const Expr *Expression, + const TargetInfo &Target) { + Expression = Expression->IgnoreImpCasts(); + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { +// Handle cases where the expression is a variable +const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); +if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) +if (Target.isValidGCCRegisterName(Attr->getLabel())) + return Target.getNormalizedGCCRegisterName(Attr->getLabel()); +} + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list. If there's a conflict, returns the location of the +// conflicted clobber, else returns nullptr +SourceLocation +GetClobberConflictLocation(MultiExprArg Exprs, StringLiteral **Constraints, + StringLiteral **Clobbers, int NumClobbers, + const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm + // statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { +StringRef Constraint = Constraints[i]->getString(); +StringRef InOutReg = Target.getConstraintRegister( +Constraint, ExtractRegisterName(Exprs[i], Target)); +if (InOutReg != "") + InOutVars.insert(InOutReg); + } + // Check for each item in the clobber list if it conflicts with the input + // or output + for (int i = 0; i < NumClobbers; ++i) { +StringRef Clobber = Clobbers[i]->getString(); +// We only check registers, therefore we don't check cc and memory +// clobbers +if (Clobber == "cc" || Clobber == "memory") + continue; +Clobber = Target.getNormalizedGCCRegisterName(Clobber, true); +// Go over the output's registers we collected +if (InOutVars.count(Clobber)) + return Clobbers[i]->getLocStart(); + } + return SourceLocation(); +} + StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, bool IsVolatile, unsigned NumOutputs, unsigned NumInputs, IdentifierInfo **Names, @@ -543,6 +595,13 @@ return StmtError();
[PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar updated this revision to Diff 74500. zizhar added a comment. changed :) Thanks for the review :> https://reviews.llvm.org/D15075 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Basic/Targets.cpp lib/Headers/intrin.h lib/Sema/SemaStmtAsm.cpp test/Sema/asm.c Index: test/Sema/asm.c === --- test/Sema/asm.c +++ test/Sema/asm.c @@ -28,6 +28,16 @@ asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010 Index: lib/Sema/SemaStmtAsm.cpp === --- lib/Sema/SemaStmtAsm.cpp +++ lib/Sema/SemaStmtAsm.cpp @@ -22,6 +22,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -137,6 +138,57 @@ return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +static StringRef extractRegisterName(const Expr *Expression, + const TargetInfo &Target) { + Expression = Expression->IgnoreImpCasts(); + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { +// Handle cases where the expression is a variable +const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); +if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) +if (Target.isValidGCCRegisterName(Attr->getLabel())) + return Target.getNormalizedGCCRegisterName(Attr->getLabel(), true); +} + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list. If there's a conflict, returns the location of the +// conflicted clobber, else returns nullptr +static SourceLocation +getClobberConflictLocation(MultiExprArg Exprs, StringLiteral **Constraints, + StringLiteral **Clobbers, int NumClobbers, + const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm + // statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { +StringRef Constraint = Constraints[i]->getString(); +StringRef InOutReg = Target.getConstraintRegister( +Constraint, extractRegisterName(Exprs[i], Target)); +if (InOutReg != "") + InOutVars.insert(InOutReg); + } + // Check for each item in the clobber list if it conflicts with the input + // or output + for (int i = 0; i < NumClobbers; ++i) { +StringRef Clobber = Clobbers[i]->getString(); +// We only check registers, therefore we don't check cc and memory +// clobbers +if (Clobber == "cc" || Clobber == "memory") + continue; +Clobber = Target.getNormalizedGCCRegisterName(Clobber, true); +// Go over the output's registers we collected +if (InOutVars.count(Clobber)) + return Clobbers[i]->getLocStart(); + } + return SourceLocation(); +} + StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, bool IsVolatile, unsigned NumOutputs, unsigned NumInputs, IdentifierInfo **Names, @@ -543,6 +595,13 @@ return StmtError();
Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar updated this revision to Diff 70660. zizhar added a comment. Hi, I have moved the relevant code to TargetInfo as previously discussed, and created a default implementation and the x86 implementation. Regarding the intrinsics, I believe it should be a separate fix, as it is not related to the clobber conflict this review is about. Do you want me to open a bugzilla on the intrinsics? https://reviews.llvm.org/D15075 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Basic/Targets.cpp lib/Headers/intrin.h lib/Sema/SemaStmtAsm.cpp test/Sema/asm.c Index: test/Sema/asm.c === --- test/Sema/asm.c +++ test/Sema/asm.c @@ -28,6 +28,16 @@ asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010 Index: lib/Sema/SemaStmtAsm.cpp === --- lib/Sema/SemaStmtAsm.cpp +++ lib/Sema/SemaStmtAsm.cpp @@ -22,6 +22,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -137,6 +138,55 @@ return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +StringRef ExtractRegisterName(const Expr *Expression, const TargetInfo &Target) { + while (const ImplicitCastExpr *P = dyn_cast(Expression)) { +Expression = P->getSubExpr(); + } + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { +// Handle cases where the expression is a variable +const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); +if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) +return Target.isValidGCCRegisterName(Attr->getLabel()) +? Target.getNormalizedGCCRegisterName(Attr->getLabel(), true) +: ""; +} + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list +bool HasClobberConflict(MultiExprArg Exprs, StringLiteral **Constraints, + StringLiteral **Clobbers, int NumClobbers, + const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { +StringRef Constraint = Constraints[i]->getString(); +StringRef InOutReg = Target.getConstraintRegister( + Constraint, ExtractRegisterName(Exprs[i], Target)); +if (InOutReg != "") + InOutVars.insert(InOutReg); + } + // Check for each item in the clobber list if it conflicts with the input + // or output + for (int i = 0; i < NumClobbers; ++i) { +StringRef Clobber = Clobbers[i]->getString(); +// We only check registers, therefore we don't check cc and memory clobbers +if (Clobber == "cc" || Clobber == "memory") + continue; +Clobber = Target.getNormalizedGCCRegisterName(Clobber, true); +// Go over the output's registers we collected +if (InOutVars.find(Clobber) != InOutVars.end()) + return true; + } + return false; +} + StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, bool IsVolatile, unsigned NumOutputs,
Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar added a comment. You understood corectly :) After going a bit through the log, I think that there is no reason for clang to not detect it, probably the check was just forgotten. This patch is the check. "r" constraint means (taken from the spec:) A register operand is allowed provided that it is in a general register. ("%rcx") means - use the register rcx. http://reviews.llvm.org/D15075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar updated this revision to Diff 60996. zizhar added a comment. A redo, investigated a bit more, had to fix some things, moved it to ActOnGCCAsmStmt. fixed intrin.h (the patch reveals a problem in it). added tests to check for regression and examples. http://reviews.llvm.org/D15075 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Headers/Intrin.h lib/Sema/SemaStmtAsm.cpp test/Sema/asm.c Index: test/Sema/asm.c === --- test/Sema/asm.c +++ test/Sema/asm.c @@ -28,6 +28,16 @@ asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010 Index: lib/Sema/SemaStmtAsm.cpp === --- lib/Sema/SemaStmtAsm.cpp +++ lib/Sema/SemaStmtAsm.cpp @@ -23,6 +23,7 @@ #include "clang/Sema/ScopeInfo.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/BitVector.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -138,6 +139,94 @@ return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +StringRef ExtractRegisterName(const Expr *Expression, + const TargetInfo &Target) { + while (const ImplicitCastExpr *P = dyn_cast(Expression)) { +Expression = P->getSubExpr(); + } + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { +// Handle cases where the expression is a variable +const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); +if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) +return Target.isValidGCCRegisterName(Attr->getLabel()) +? Target.getNormalizedGCCRegisterName(Attr->getLabel(), true) +: ""; +} + } + return ""; +} + +// Gets a constraint and returns the register the constraint specifies or "" if +// there is no register associated with the constraint +StringRef GetConstraintRegister(const StringRef &Constraint, + const Expr *Expression, + const TargetInfo &Target) { + for (int i = 0; i < Constraint.size(); ++i) { +switch (Constraint[i]) { + // Ignore these +case '*': +case '?': +case '!': + // Will see this and the following in mult-alt constraints. +case '=': +case '+': + continue; + // For the register constraints, return the matching register name +case 'a': + return "ax"; +case 'b': + return "bx"; +case 'c': + return "cx"; +case 'd': + return "dx"; +case 'S': + return "si"; +case 'D': + return "di"; + // In case the constraint is 'r' we need to extract the register name +case 'r': + return ExtractRegisterName(Expression, Target); +default: + // Default value if there is no constraint for the register + return ""; +} + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list +bool HasClobberConflict(MultiExprArg Exprs, StringLiteral **Constraints, + StringLiteral **Clobbers, int NumClobbers, + const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { +StringRef Constraint = Constrai
Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar added a comment. Hello Hans, Regarding the changes in intrin.h, You're the last to change/add these lines, I found out that there is a conflict between the clobber list and input or output lists. Can you review this change? Thanks, Ziv Izhar https://reviews.llvm.org/D15075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list
zizhar added a comment. Akira, You've mentioned a good point, this X86 logic should indeed be moved to X86TargetInfo. The current convertConstraint() implementation is not doing what I need – it doesn’t handle all the input/output constraints I need, and it returns the constraint in a different format than the one I need. E.g. convertConstraint() does not handle “r” constraints or special characters like “=”, “+”, also, the function returns the registers as “{ax}”, when I need the “ax”. I think it is better to add a new function for my logic instead of trying to adjust this function to handle both its old logic and my new logic. My new solution is going to be to create another virtual function in TargetInfo that will do everything I need. The code that is currently under GetConstraintRegister() and ExtractRegisterName() will now be part of the X86TargetInfo implementation of this virtual function. For all the other architectures’ TargetInfos I’ll create a default implementation that will return an empty string and they can implement it if they want to (until they do, the existing behavior will be retained). Can I have your opinion on this? Do you see a better way to implement this? Thanks, Ziv Izhar. https://reviews.llvm.org/D15075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits