Page MenuHomePhabricator

Integrated assembler. :vararg macro.

Authored by dyatkovskiy on Apr 11 2014, 5:18 AM.



Please review fix for PR18919, that adds support for :vararg macro in integrated assembler.

Diff Detail

Event Timeline

rnk added a comment.Apr 18 2014, 1:33 PM

IIRC David was hacking on some gnu assembly parsing stuff.

I'd like to see more tests here. .macro statements with normal arguments preceding the :vararg one. Something like:

.macro foo bar, baz:vararg

A test without a comma between bar and baz would be good as well.


This comment is a bit hard to read, perhaps something more like:

// We want the quotes around the string's contents when parsing for varargs.

Please use /*Varag=*/false to make it clear what is going on.

dyatkovskiy updated this revision to Unknown Object (????).Apr 20 2014, 11:03 PM

Thank you guys!
Everything has been fixed according to your remarks:

  • Fixed comments
  • Added new tests
  • I have also found small bug (thank you for test proposals again) and fixed it too.
dyatkovskiy added inline comments.Apr 20 2014, 11:08 PM

Sorry for extra lines. Of course they will be removed!

You should probably have a test with a darwin triple too. I cced Jim Grosbach for him to comment if this is a desirable feature for darwin or not. I know the macro system is a bit different, but I don't know the details.

Hi Rafael,
I have implementation for darwin, though in draft state.
There are few reasons/questions:

  1. I didn't find anything about vararg in darwin assembler reference. Or even some .s examples.
  2. If I remember properly, you can substitude parameters by their number: $0, $1, etc. So, may he then, you can refer to some of vararg params as well? Am I right here?
  3. Can I use whole varargs set substitution? Like in Linux tests?
  4. If answer to #2 and #3 is "yes", then I suspect we have to keep vararg values in two forms: as parsed and separated values, and as single string. We need second form, because we need to remember separators as well, in another words we have to expand it as-is. At least we have to keep separators for second form.

Anyway I'm ready to work on it too. But darwin support could contain a bit more changes. Perhaps we could commit it separately?

dyatkovskiy updated this revision to Unknown Object (????).Apr 21 2014, 11:38 PM

Please review updated patch.

  • Added test, that checks :vararg state for darwin.


rafael accepted this revision.Apr 22 2014, 7:17 AM

LGTM with nits.


cc is not needed in this test.


cc is never 0 in this test, do you need it?

dyatkovskiy closed this revision.Apr 23 2014, 11:12 PM

Thanks for reviews!