[PATCH] [ms-inline asm] Support offsets after segment registers
Reid Kleckner
rnk at google.com
Mon Aug 26 12:48:51 PDT 2013
Sorry, I should've given more feedback. It wasn't just that one comment, it's more that it took me a long time to figure out how this is supposed to work, and the comment was just the easiest thing to point at.
I think we really need to phrase it as "is this the end of an operand?", at which point we can build a complete memop
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1430
@@ +1429,3 @@
+/// \brief Parse intel style segment offset.
+X86Operand *X86AsmParser::TryParseIntelSegmentOffset(unsigned SegReg,
+ int64_t ImmDisp,
----------------
This doesn't seem named well, maybe this should be something like ParseEndOfOperand? PeekEndOfOperand? This isn't really about parsing seg:offs.
================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1436
@@ +1435,3 @@
+ getLexer().is(AsmToken::EndOfStatement))) {
+ // Handle things like INST SegReg : Imm, SRC
+ // and INST DST, SegReg : Imm
----------------
This comment is confusing to me because we're not actually parsing segments, colons, or immediates. I think rewriting around the idea of "is this operand complete?" may help.
http://llvm-reviews.chandlerc.com/D1470
More information about the llvm-commits
mailing list