Page MenuHomePhabricator

[ms] [llvm-ml] Enable support for MASM-style macro procedures
ClosedPublic

Authored by epastor on Oct 19 2020, 1:51 PM.

Details

Summary

Allows the MACRO directive to define macro procedures with parameters and macro-local symbols.

Supports required and optional parameters (including default values), and matches ml64.exe for its macro-local symbol handling (up to 65536 macro-local symbols in any translation unit).

Diff Detail

Unit TestsFailed

TimeTest
370 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp
190 mswindows > lld.ELF/invalid::symtab-sh-info.s
Script: -- : 'RUN: at line 4'; c:\ws\w64\llvm-project\premerge-checks\build\bin\yaml2obj.exe --docnum=1 C:\ws\w64\llvm-project\premerge-checks\lld\test\ELF\invalid\symtab-sh-info.s -o C:\ws\w64\llvm-project\premerge-checks\build\tools\lld\test\ELF\invalid\Output\symtab-sh-info.s.tmp.o

Event Timeline

epastor created this revision.Oct 19 2020, 1:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2020, 1:51 PM
epastor requested review of this revision.Oct 19 2020, 1:51 PM
thakis accepted this revision.Nov 2 2020, 4:21 PM

Basically lg. Please add some tests for the error cases.

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

why not just unsigned?

2259

did the altmacro call here disappear intentionally? probably, but delete parseDirectiveAltmacro()'s definition too then?

2388

why is this moving down here?

2609–2610

comment is now outdated

2674

is this guaranteed to be in bounds?

This revision is now accepted and ready to land.Nov 2 2020, 4:21 PM
epastor updated this revision to Diff 302582.Nov 3 2020, 7:43 AM
epastor marked 5 inline comments as done.

Addressing feedback

Still TBD: adding tests for the error cases

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

Attempting to match ML.EXE; it uses an explicitly 16-bit counter to label its local symbols, wrapping (and thus failing with duplicate symbol definitions) if more than that many local symbols are defined.

2259

Yes, this is intentional. Done.

2388

Because it isn't just moving; this section is used for MASM's infix directives, which includes MACRO. (e.g., you don't start a line with MACRO, you write <macroName> MACRO.)

2609–2610

Removed.

2674

... no. This was fixed later in the stack, and I forgot to backport it. Thanks for catching!

epastor updated this revision to Diff 302586.Nov 3 2020, 8:06 AM

Adding tests for the error cases

This revision was landed with ongoing or failed builds.Nov 4 2020, 7:30 AM
This revision was automatically updated to reflect the committed changes.