[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