Page MenuHomePhabricator

[ms] [llvm-ml] Support command-line defines
ClosedPublic

Authored by epastor on Oct 23 2020, 10:49 AM.

Details

Summary

Enable command-line defines as textmacros

Diff Detail

Event Timeline

epastor created this revision.Oct 23 2020, 10:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 23 2020, 10:49 AM
epastor requested review of this revision.Oct 23 2020, 10:49 AM
epastor updated this revision to Diff 303174.Nov 5 2020, 10:33 AM

Rebase on HEAD

epastor updated this revision to Diff 303185.Nov 5 2020, 10:44 AM

Rebase on parent

thakis accepted this revision.Nov 6 2020, 6:43 AM
thakis added a subscriber: thakis.

lg with tweaks:

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

masm doesn't do it, but I think we should diag on redefinitions in -D flags, at least if the redefinition has a different value. that's likely always a bug in the build system invoking us.

llvm/test/tools/llvm-ml/command_line_defines.asm
16

Can you test ifdef elseifdef too?

This revision is now accepted and ready to land.Nov 6 2020, 6:43 AM
epastor updated this revision to Diff 303979.Nov 9 2020, 1:32 PM
epastor marked an inline comment as done.

Add ifdef testing, and fix a minor StringRef vs. std::string issue.

epastor updated this revision to Diff 307163.Nov 23 2020, 12:09 PM

Rebase on parent

epastor updated this revision to Diff 307168.Nov 23 2020, 12:19 PM

Rebase on parent

epastor added inline comments.Nov 23 2020, 12:20 PM
llvm/lib/MC/MCParser/MasmParser.cpp
6917

I'm not sure there's a clean way to do this that isn't an error, and I'd like to maintain compatibility with ml64.exe on this. If you think it's worth special-casing this to plumb a warning through, we can do that... but I'm not convinced it's a good idea.

epastor updated this revision to Diff 307187.Nov 23 2020, 1:32 PM

Rebase on parent

epastor updated this revision to Diff 307690.Nov 25 2020, 12:35 PM

Rebase on parent

epastor updated this revision to Diff 308782.Dec 1 2020, 2:48 PM

Rebase on HEAD

This revision was landed with ongoing or failed builds.Dec 1 2020, 3:06 PM
This revision was automatically updated to reflect the committed changes.
thakis added inline comments.Dec 1 2020, 3:07 PM
llvm/lib/MC/MCParser/MasmParser.cpp
6917

It'd match clang, which would be good, no?