This is an archive of the discontinued LLVM Phabricator instance.

[ms] [llvm-ml] Add support for numeric built-in symbols
ClosedPublic

Authored by epastor on Jun 25 2021, 7:46 PM.

Details

Summary

Support @Version and @Line as built-in symbols. For now, resolves @Version to 1427 (the same as for the VS 2019 release of ML.EXE).

Diff Detail

Event Timeline

epastor created this revision.Jun 25 2021, 7:46 PM
epastor requested review of this revision.Jun 25 2021, 7:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2021, 7:46 PM

Can you check if ml.exe allows defining a local variable overriding e.g. @Line? What happens then? Does it error? Can you add a test for that?

(…other that that, lg)

epastor updated this revision to Diff 356181.Jul 2 2021, 8:54 AM

Error on redefinition of built-in symbols (matching ML.EXE)

Can you check if ml.exe allows defining a local variable overriding e.g. @Line? What happens then? Does it error? Can you add a test for that?

Confirmed that ml.exe errors in this case; matched this behavior, and added corresponding test.

epastor updated this revision to Diff 356219.Jul 2 2021, 10:54 AM

Rebase on parent

epastor updated this revision to Diff 356220.Jul 2 2021, 10:59 AM

Rebase on parent

epastor updated this revision to Diff 356221.Jul 2 2021, 11:02 AM

Fix an accidental import of MasmParser.cpp from a child commit

epastor updated this revision to Diff 356222.Jul 2 2021, 11:05 AM

Fix accidental fold with the child commit

epastor updated this revision to Diff 356237.Jul 2 2021, 11:50 AM

Fix symbol usage bug

thakis accepted this revision.Jul 5 2021, 7:14 PM

lg

llvm/lib/MC/MCParser/MasmParser.cpp
3516

Why is this changing? If it's intentional, nit: Named parameter comment style in LLVM is /*SetUsed=*/false (ie with = and without space)

llvm/test/tools/llvm-ml/variable_redef_errors.asm
11

Can you add a line for @Version too?

This revision is now accepted and ready to land.Jul 5 2021, 7:14 PM
epastor updated this revision to Diff 358035.Jul 12 2021, 12:42 PM
epastor marked 2 inline comments as done.

Addressed feedback (and rebased on HEAD)

epastor added inline comments.Jul 21 2021, 6:55 AM
llvm/lib/MC/MCParser/MasmParser.cpp
3516

We're removing Var.NumericValue because it breaks the parallels between Variables and BuiltinSymbols. Instead, we rely on the value stored in the symbol itself as authoritative - and if it's not an MCConstantExpr, then it's definitely not a MASM variable.

Fixed the named parameter comment style, though I believe this was copied from AsmParser.cpp originally...

This revision was landed with ongoing or failed builds.Jul 21 2021, 8:43 AM
This revision was automatically updated to reflect the committed changes.