[cfe-commits] r162132 - in /cfe/trunk/lib/Sema: CMakeLists.txt SemaStmt.cpp SemaStmtAsm.cpp
Chad Rosier
mcrosier at apple.com
Wed Aug 22 20:51:18 PDT 2012
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 <silvas at purdue.edu> 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 <mcrosier at apple.com> 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
More information about the cfe-commits
mailing list