Sean, Thanks for pointing this out. Renaming those induction variables to improve readability was on my todo list anyways, so I'll take care of it tomorrow.
Chad On Aug 22, 2012, at 6:37 PM, Sean Silva <[email protected]> wrote: > + for (unsigned i = 0, e = IDVal.size(); i != e; ++i) > + Opcode.push_back(tolower(IDVal[i])); > > Hey Chad, sorry for commenting on this rather old email, but this use > of 'i' as a variable name inside a loop whose index variable is > already 'i' seems questionable, especially since you're using 'j' for > another inner loop. I discovered this because GCC just gave me an odd > warning over it: > > SemaStmtAsm.cpp:594:63: warning: name lookup of ‘i’ changed [enabled by > default] > SemaStmtAsm.cpp:509:17: warning: matches this ‘i’ under ISO standard > rules [enabled by default] > SemaStmtAsm.cpp:536:19: warning: matches this ‘i’ under old rules > [enabled by default] > > IMO this warning is pointless, but nonetheless, I think it would be > good to change the loop variable to be 'j'. > > --Sean Silva > > On Fri, Aug 17, 2012 at 5:19 PM, Chad Rosier <[email protected]> wrote: >> Author: mcrosier >> Date: Fri Aug 17 16:19:40 2012 >> New Revision: 162132 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=162132&view=rev >> Log: >> [ms-inline asm] Extract AsmStmt handling into a separate file, so as to not >> pollute SemaStmt with extraneous asm handling logic. >> >> Added: >> cfe/trunk/lib/Sema/SemaStmtAsm.cpp >> Modified: >> cfe/trunk/lib/Sema/CMakeLists.txt >> cfe/trunk/lib/Sema/SemaStmt.cpp >> >> Modified: cfe/trunk/lib/Sema/CMakeLists.txt >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/CMakeLists.txt?rev=162132&r1=162131&r2=162132&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/CMakeLists.txt (original) >> +++ cfe/trunk/lib/Sema/CMakeLists.txt Fri Aug 17 16:19:40 2012 >> @@ -39,6 +39,7 @@ >> SemaOverload.cpp >> SemaPseudoObject.cpp >> SemaStmt.cpp >> + SemaStmtAsm.cpp >> SemaStmtAttr.cpp >> SemaTemplate.cpp >> SemaTemplateDeduction.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=162132&r1=162131&r2=162132&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Fri Aug 17 16:19:40 2012 >> @@ -28,27 +28,10 @@ >> #include "clang/Lex/Preprocessor.h" >> #include "clang/Basic/TargetInfo.h" >> #include "llvm/ADT/ArrayRef.h" >> -#include "llvm/ADT/BitVector.h" >> #include "llvm/ADT/STLExtras.h" >> #include "llvm/ADT/SmallPtrSet.h" >> #include "llvm/ADT/SmallString.h" >> #include "llvm/ADT/SmallVector.h" >> -#include "llvm/ADT/Triple.h" >> -#include "llvm/MC/MCAsmInfo.h" >> -#include "llvm/MC/MCContext.h" >> -#include "llvm/MC/MCInst.h" >> -#include "llvm/MC/MCInstPrinter.h" >> -#include "llvm/MC/MCInstrInfo.h" >> -#include "llvm/MC/MCObjectFileInfo.h" >> -#include "llvm/MC/MCRegisterInfo.h" >> -#include "llvm/MC/MCStreamer.h" >> -#include "llvm/MC/MCSubtargetInfo.h" >> -#include "llvm/MC/MCTargetAsmParser.h" >> -#include "llvm/MC/MCParser/MCAsmLexer.h" >> -#include "llvm/MC/MCParser/MCAsmParser.h" >> -#include "llvm/Support/SourceMgr.h" >> -#include "llvm/Support/TargetRegistry.h" >> -#include "llvm/Support/TargetSelect.h" >> using namespace clang; >> using namespace sema; >> >> @@ -2485,600 +2468,6 @@ >> return Owned(Result); >> } >> >> -/// CheckAsmLValue - GNU C has an extremely ugly extension whereby they >> silently >> -/// ignore "noop" casts in places where an lvalue is required by an inline >> asm. >> -/// We emulate this behavior when -fheinous-gnu-extensions is specified, but >> -/// provide a strong guidance to not use it. >> -/// >> -/// This method checks to see if the argument is an acceptable l-value and >> -/// returns false if it is a case we can handle. >> -static bool CheckAsmLValue(const Expr *E, Sema &S) { >> - // Type dependent expressions will be checked during instantiation. >> - if (E->isTypeDependent()) >> - return false; >> - >> - if (E->isLValue()) >> - return false; // Cool, this is an lvalue. >> - >> - // Okay, this is not an lvalue, but perhaps it is the result of a cast >> that we >> - // are supposed to allow. >> - const Expr *E2 = E->IgnoreParenNoopCasts(S.Context); >> - if (E != E2 && E2->isLValue()) { >> - if (!S.getLangOpts().HeinousExtensions) >> - S.Diag(E2->getLocStart(), diag::err_invalid_asm_cast_lvalue) >> - << E->getSourceRange(); >> - else >> - S.Diag(E2->getLocStart(), diag::warn_invalid_asm_cast_lvalue) >> - << E->getSourceRange(); >> - // Accept, even if we emitted an error diagnostic. >> - return false; >> - } >> - >> - // None of the above, just randomly invalid non-lvalue. >> - return true; >> -} >> - >> -/// isOperandMentioned - Return true if the specified operand # is mentioned >> -/// anywhere in the decomposed asm string. >> -static bool isOperandMentioned(unsigned OpNo, >> - ArrayRef<AsmStmt::AsmStringPiece> AsmStrPieces) { >> - for (unsigned p = 0, e = AsmStrPieces.size(); p != e; ++p) { >> - const AsmStmt::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; >> -} >> - >> -StmtResult Sema::ActOnAsmStmt(SourceLocation AsmLoc, bool IsSimple, >> - bool IsVolatile, unsigned NumOutputs, >> - unsigned NumInputs, IdentifierInfo **Names, >> - MultiExprArg constraints, MultiExprArg exprs, >> - Expr *asmString, MultiExprArg clobbers, >> - SourceLocation RParenLoc, bool MSAsm) { >> - unsigned NumClobbers = clobbers.size(); >> - StringLiteral **Constraints = >> - reinterpret_cast<StringLiteral**>(constraints.get()); >> - Expr **Exprs = exprs.get(); >> - StringLiteral *AsmString = cast<StringLiteral>(asmString); >> - StringLiteral **Clobbers = >> reinterpret_cast<StringLiteral**>(clobbers.get()); >> - >> - SmallVector<TargetInfo::ConstraintInfo, 4> OutputConstraintInfos; >> - >> - // The parser verifies that there is a string literal here. >> - if (!AsmString->isAscii()) >> - return >> StmtError(Diag(AsmString->getLocStart(),diag::err_asm_wide_character) >> - << AsmString->getSourceRange()); >> - >> - for (unsigned i = 0; i != NumOutputs; i++) { >> - StringLiteral *Literal = Constraints[i]; >> - if (!Literal->isAscii()) >> - return >> StmtError(Diag(Literal->getLocStart(),diag::err_asm_wide_character) >> - << Literal->getSourceRange()); >> - >> - StringRef OutputName; >> - if (Names[i]) >> - OutputName = Names[i]->getName(); >> - >> - TargetInfo::ConstraintInfo Info(Literal->getString(), OutputName); >> - if (!Context.getTargetInfo().validateOutputConstraint(Info)) >> - return StmtError(Diag(Literal->getLocStart(), >> - diag::err_asm_invalid_output_constraint) >> - << Info.getConstraintStr()); >> - >> - // Check that the output exprs are valid lvalues. >> - Expr *OutputExpr = Exprs[i]; >> - if (CheckAsmLValue(OutputExpr, *this)) { >> - return StmtError(Diag(OutputExpr->getLocStart(), >> - diag::err_asm_invalid_lvalue_in_output) >> - << OutputExpr->getSourceRange()); >> - } >> - >> - OutputConstraintInfos.push_back(Info); >> - } >> - >> - SmallVector<TargetInfo::ConstraintInfo, 4> InputConstraintInfos; >> - >> - for (unsigned i = NumOutputs, e = NumOutputs + NumInputs; i != e; i++) { >> - StringLiteral *Literal = Constraints[i]; >> - if (!Literal->isAscii()) >> - return >> StmtError(Diag(Literal->getLocStart(),diag::err_asm_wide_character) >> - << Literal->getSourceRange()); >> - >> - StringRef InputName; >> - if (Names[i]) >> - InputName = Names[i]->getName(); >> - >> - TargetInfo::ConstraintInfo Info(Literal->getString(), InputName); >> - if >> (!Context.getTargetInfo().validateInputConstraint(OutputConstraintInfos.data(), >> - NumOutputs, Info)) { >> - return StmtError(Diag(Literal->getLocStart(), >> - diag::err_asm_invalid_input_constraint) >> - << Info.getConstraintStr()); >> - } >> - >> - Expr *InputExpr = Exprs[i]; >> - >> - // Only allow void types for memory constraints. >> - if (Info.allowsMemory() && !Info.allowsRegister()) { >> - if (CheckAsmLValue(InputExpr, *this)) >> - return StmtError(Diag(InputExpr->getLocStart(), >> - diag::err_asm_invalid_lvalue_in_input) >> - << Info.getConstraintStr() >> - << InputExpr->getSourceRange()); >> - } >> - >> - if (Info.allowsRegister()) { >> - if (InputExpr->getType()->isVoidType()) { >> - return StmtError(Diag(InputExpr->getLocStart(), >> - diag::err_asm_invalid_type_in_input) >> - << InputExpr->getType() << Info.getConstraintStr() >> - << InputExpr->getSourceRange()); >> - } >> - } >> - >> - ExprResult Result = DefaultFunctionArrayLvalueConversion(Exprs[i]); >> - if (Result.isInvalid()) >> - return StmtError(); >> - >> - Exprs[i] = Result.take(); >> - InputConstraintInfos.push_back(Info); >> - } >> - >> - // Check that the clobbers are valid. >> - for (unsigned i = 0; i != NumClobbers; i++) { >> - StringLiteral *Literal = Clobbers[i]; >> - if (!Literal->isAscii()) >> - return >> StmtError(Diag(Literal->getLocStart(),diag::err_asm_wide_character) >> - << Literal->getSourceRange()); >> - >> - StringRef Clobber = Literal->getString(); >> - >> - if (!Context.getTargetInfo().isValidClobber(Clobber)) >> - return StmtError(Diag(Literal->getLocStart(), >> - diag::err_asm_unknown_register_name) << Clobber); >> - } >> - >> - AsmStmt *NS = >> - new (Context) AsmStmt(Context, AsmLoc, IsSimple, IsVolatile, MSAsm, >> - NumOutputs, NumInputs, Names, Constraints, Exprs, >> - AsmString, NumClobbers, Clobbers, RParenLoc); >> - // Validate the asm string, ensuring it makes sense given the operands we >> - // have. >> - SmallVector<AsmStmt::AsmStringPiece, 8> Pieces; >> - unsigned DiagOffs; >> - if (unsigned DiagID = NS->AnalyzeAsmString(Pieces, Context, DiagOffs)) { >> - Diag(getLocationOfStringLiteralByte(AsmString, DiagOffs), DiagID) >> - << AsmString->getSourceRange(); >> - return StmtError(); >> - } >> - >> - // Validate tied input operands for type mismatches. >> - for (unsigned i = 0, e = InputConstraintInfos.size(); i != e; ++i) { >> - TargetInfo::ConstraintInfo &Info = InputConstraintInfos[i]; >> - >> - // If this is a tied constraint, verify that the output and input have >> - // either exactly the same type, or that they are int/ptr operands with >> the >> - // same size (int/long, int*/long, are ok etc). >> - if (!Info.hasTiedOperand()) continue; >> - >> - unsigned TiedTo = Info.getTiedOperand(); >> - unsigned InputOpNo = i+NumOutputs; >> - Expr *OutputExpr = Exprs[TiedTo]; >> - Expr *InputExpr = Exprs[InputOpNo]; >> - >> - if (OutputExpr->isTypeDependent() || InputExpr->isTypeDependent()) >> - continue; >> - >> - QualType InTy = InputExpr->getType(); >> - QualType OutTy = OutputExpr->getType(); >> - if (Context.hasSameType(InTy, OutTy)) >> - continue; // All types can be tied to themselves. >> - >> - // Decide if the input and output are in the same domain (integer/ptr or >> - // floating point. >> - enum AsmDomain { >> - AD_Int, AD_FP, AD_Other >> - } InputDomain, OutputDomain; >> - >> - if (InTy->isIntegerType() || InTy->isPointerType()) >> - InputDomain = AD_Int; >> - else if (InTy->isRealFloatingType()) >> - InputDomain = AD_FP; >> - else >> - InputDomain = AD_Other; >> - >> - if (OutTy->isIntegerType() || OutTy->isPointerType()) >> - OutputDomain = AD_Int; >> - else if (OutTy->isRealFloatingType()) >> - OutputDomain = AD_FP; >> - else >> - OutputDomain = AD_Other; >> - >> - // They are ok if they are the same size and in the same domain. This >> - // allows tying things like: >> - // void* to int* >> - // void* to int if they are the same size. >> - // double to long double if they are the same size. >> - // >> - uint64_t OutSize = Context.getTypeSize(OutTy); >> - uint64_t InSize = Context.getTypeSize(InTy); >> - if (OutSize == InSize && InputDomain == OutputDomain && >> - InputDomain != AD_Other) >> - continue; >> - >> - // If the smaller input/output operand is not mentioned in the asm >> string, >> - // then we can promote the smaller one to a larger input and the asm >> string >> - // won't notice. >> - bool SmallerValueMentioned = false; >> - >> - // 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)) { >> - // 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 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; >> - } >> - >> - // If the smaller value wasn't mentioned in the asm string, and if the >> - // output was a register, just extend the shorter one to the size of the >> - // larger one. >> - if (!SmallerValueMentioned && InputDomain != AD_Other && >> - OutputConstraintInfos[TiedTo].allowsRegister()) >> - continue; >> - >> - // Either both of the operands were mentioned or the smaller one was >> - // mentioned. One more special case that we'll allow: if the tied >> input is >> - // 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)) { >> - CastKind castKind = >> - (OutTy->isBooleanType() ? CK_IntegralToBoolean : CK_IntegralCast); >> - InputExpr = ImpCastExprToType(InputExpr, OutTy, castKind).take(); >> - Exprs[InputOpNo] = InputExpr; >> - NS->setInputExpr(i, InputExpr); >> - continue; >> - } >> - >> - Diag(InputExpr->getLocStart(), >> - diag::err_asm_tying_incompatible_types) >> - << InTy << OutTy << OutputExpr->getSourceRange() >> - << InputExpr->getSourceRange(); >> - return StmtError(); >> - } >> - >> - return Owned(NS); >> -} >> - >> -// isMSAsmKeyword - Return true if this is an MS-style inline asm keyword. >> These >> -// require special handling. >> -static bool isMSAsmKeyword(StringRef Name) { >> - bool Ret = llvm::StringSwitch<bool>(Name) >> - .Cases("EVEN", "ALIGN", true) // Alignment directives. >> - .Cases("LENGTH", "SIZE", "TYPE", true) // Type and variable sizes. >> - .Case("_emit", true) // _emit Pseudoinstruction. >> - .Default(false); >> - return Ret; >> -} >> - >> -static StringRef getSpelling(Sema &SemaRef, Token AsmTok) { >> - StringR _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
