Page MenuHomePhabricator

[ms] [llvm-ml] Add support for .radix directive, and accept all radix specifiers
ClosedPublic

Authored by epastor on Sep 9 2020, 11:09 AM.

Details

Summary

Add support for .radix directive, and radix specifiers [yY] (binary), [oOqQ] (octal), and [tT] (decimal).

Also, when lexing MASM integers, require radix specifier; MASM requires that all literals without a radix specifier be treated as in the default radix. (e.g., 0100 = 100)

Diff Detail

Event Timeline

epastor created this revision.Sep 9 2020, 11:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2020, 11:09 AM
epastor requested review of this revision.Sep 9 2020, 11:09 AM
thakis added inline comments.Sep 14 2020, 10:27 AM
llvm/lib/MC/MCParser/AsmLexer.cpp
403

Can't hex literals contain 'e' in non-float literals as well? how is that handled?

epastor updated this revision to Diff 291635.Sep 14 2020, 11:54 AM

Handle float literals in MASM earlier, and avoid confusing the hex digit 'e' for an exponential marker.

epastor marked an inline comment as done.Sep 14 2020, 11:58 AM
epastor added inline comments.
llvm/lib/MC/MCParser/AsmLexer.cpp
403

... that was a really good question, thank you! Apparently in MASM, float literals are always distinguished by containing a . character; it's invalid to write (e.g.) 13E1 unless the default radix is >= 15, and it will then be interpreted as an integer.

I've corrected the code.

thakis added inline comments.Sep 14 2020, 12:59 PM
llvm/lib/MC/MCParser/AsmLexer.cpp
403

Thanks! Can you add a test that would've failed with the old code but that passes with the new code?

epastor updated this revision to Diff 292302.Sep 16 2020, 12:04 PM
epastor marked an inline comment as done.

Fix error in float literal lexing, and improve tests.

Also, improve lexer error handling for X86AsmParser in Intel expressions.

epastor marked an inline comment as done.Sep 16 2020, 12:09 PM
thakis accepted this revision.Sep 18 2020, 6:38 AM

lg assuming we have tests for float literals containing an e in them somewhere already.

This revision is now accepted and ready to land.Sep 18 2020, 6:38 AM
This revision was landed with ongoing or failed builds.Sep 23 2020, 10:46 AM
This revision was automatically updated to reflect the committed changes.