Page MenuHomePhabricator

Refactor and cleanup Assembly Parsing / Lexing
ClosedPublic

Authored by niravd on May 5 2016, 6:47 PM.

Details

Summary

Add explicit Comment Token in Assembly Lexing for future support for
outputting explicit comments from inline assembly. As part of this,
CPPHash Directives are now explicitly distinguished from Hash line
comments in Lexer.

Line comments are recorded as EndOfStatement tokens, not Comment tokens
to simplify compatibility with current TargetParsers. This slightly
complicates comment output.

This remove all lexing tasks out of the parser, does minor cleanup
to remove extraneous newlines Asm Output, and some improvements white
space handling.

Diff Detail

Repository
rL LLVM

Event Timeline

niravd updated this revision to Diff 56378.May 5 2016, 6:47 PM
niravd retitled this revision from to Refactor and cleanup Assembly Parsing / Lexing.
niravd updated this object.
niravd added reviewers: rtrieu, dwmw2.
niravd added a subscriber: llvm-commits.
niravd updated this revision to Diff 57661.May 18 2016, 12:28 PM

Modifications to keep line comments in correct place with TargetParser

niravd updated this object.May 18 2016, 12:29 PM
niravd updated this object.
niravd updated this revision to Diff 59441.Jun 2 2016, 12:04 PM

Update to fix CPP Hash Directive parsing and add testcase

niravd added a reviewer: rnk.Jun 6 2016, 6:30 AM
rnk edited edge metadata.Jun 8 2016, 11:26 AM

Overall seems like a good cleanup of a janky parser. =/

include/llvm/MC/MCParser/AsmLexer.h
32–33 ↗(On Diff #59441)

These should both follow LLVM style, i.e. leading upper case for variables & fields.

lib/MC/MCParser/AsmLexer.cpp
53 ↗(On Diff #59441)

Removing this attempt to optimize seems fine.

500 ↗(On Diff #59441)

Follow the LLVM naming conventions please.

lib/MC/MCParser/AsmParser.cpp
630 ↗(On Diff #59441)

Really it skips them, it doesn't parse them.

niravd updated this revision to Diff 60080.Jun 8 2016, 12:24 PM
niravd marked 3 inline comments as done.
niravd edited edge metadata.

Revert enum as requested in D20020, fix LLVM style issues

niravd marked an inline comment as done.Jun 13 2016, 5:52 AM

Everything appears to be resolved. Can I get a LGTM?

rnk accepted this revision.Jun 16 2016, 8:43 AM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Jun 16 2016, 8:43 AM
This revision was automatically updated to reflect the committed changes.

Please note https://llvm.org/bugs/show_bug.cgi?id=28921, which has a testcase bisected to this revision.