coby added inline comments.
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1310
}
-
-bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End)
{
+bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+ bool &ReplaceEnumIdentifier) {
----------------
blank line omitted
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1378
+ // Check if the parsed identifier was a constant Integer. Here we
+ // assume Val is of type MCConstantExpr only when it is safe to replace
+ // the identifier with its constant value.
----------------
assumption ~~> assertion
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1380
+ // the identifier with its constant value.
+ if (const MCConstantExpr *CE =
+ dyn_cast_or_null<const MCConstantExpr>(Val)) {
----------------
I think this whole section better suites within its own function. something
like 'ParseInlineAsmEnumValue'
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1383
+ StringRef ErrMsg;
+ // SM should treat the value as it would an explicit integer in the
+ // expression.
----------------
rephrase
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1385
+ // expression.
+ if(SM.onInteger(CE->getValue(), ErrMsg))
+ return Error(IdentLoc, ErrMsg);
----------------
clang format
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1388
+ // In case we are called on a bracketed expression,
+ if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+ // A single rewrite of the integer value is preformed for each enum
----------------
'isParsingInlineAsm()' is unnecessary here (you can only reach this piece of
code when parsing inline asm)
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1826
}
-
- // Rewrite the type operator and the C or C++ type or variable in terms of an
- // immediate. E.g. TYPE foo -> $$4
- unsigned Len = End.getPointer() - TypeLoc.getPointer();
- InstInfo->AsmRewrites->emplace_back(AOK_Imm, TypeLoc, Len, CVal);
-
+ // Only when in bracketed mode, preform explicit rewrite
+ if (AddImmPrefix) {
----------------
Not keen to the use of SM.getAddImmPrefix() as a mean of distinguish whether we
are parsing a bracketed expression. I know it is currently turned on when
parsing it, but it isn't asserted/guaranteed.
Regardless - I'm pretty sure we can manage without this rewrite, or at the very
least - should, now that TYPE/LENGTH/SIZE are part of the State Machine.
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1907
unsigned Len = Tok.getLoc().getPointer() - Start.getPointer();
- if (StartTok.getString().size() == Len)
- // Just add a prefix if this wasn't a complex immediate expression.
- InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Start);
- else
- // Otherwise, rewrite the complex expression as a single immediate.
+ if (StartTok.getString().size() != Len || ReplaceEnumIdentifier)
+ // Rewrite the complex expression as a single immediate.
----------------
you may just perform an AOK_Imm rewrite regardless the complexity of the
immediate expression, and neglect 'ReplaceEnumIdentifier'
Repository:
rL LLVM
https://reviews.llvm.org/D33278
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
