This is an archive of the discontinued LLVM Phabricator instance.

[MC] Support .ds directives in assembler parser
ClosedPublic

Authored by phosek on Sep 19 2016, 12:20 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

phosek updated this revision to Diff 71866.Sep 19 2016, 12:20 PM
phosek retitled this revision from to [MC] Support .ds directives in assembler parser.
phosek updated this object.
phosek added a reviewer: echristo.
phosek set the repository for this revision to rL LLVM.
phosek added subscribers: llvm-commits, phosek.
davide added a subscriber: davide.Sep 19 2016, 1:31 PM

Some comments, hope you don't mind :)

lib/MC/MCParser/AsmParser.cpp
1919–1934 ↗(On Diff #71866)

you can probably merge DK_DS_X and DK_DS_P cases (and you can do similarly for others, i.e.

case DK_DS_X:
case DK_DS_P:
  return ...
4244–4245 ↗(On Diff #71866)

Do you want to emit a diagnostic here?

4248–4249 ↗(On Diff #71866)

Please add a test for this case.

4252–4254 ↗(On Diff #71866)

and this one.

phosek updated this revision to Diff 71999.Sep 20 2016, 4:37 PM
phosek removed rL LLVM as the repository for this revision.
phosek marked 3 inline comments as done.
phosek set the repository for this revision to rL LLVM.Sep 20 2016, 5:33 PM

this looks good after my first round of comments. Thanks

davide accepted this revision.Sep 20 2016, 8:40 PM
davide added a reviewer: davide.
This revision is now accepted and ready to land.Sep 20 2016, 8:40 PM
phosek updated this revision to Diff 72367.Sep 23 2016, 2:59 PM
phosek edited edge metadata.

The patch was rebased.

This revision was automatically updated to reflect the committed changes.