Page MenuHomePhabricator

Darwin vararg parameters support in assembler macros
Needs ReviewPublic

Authored by dyatkovskiy on Apr 25 2014, 5:00 AM.



This patch implements experimental vararg support for Darwin.

It's experimental because I don't know how exaclty you want to treat such params. Perhaps you also would like to use number references for it $0, $1, .., $n.
Patch adds dual representation of vararg parameter:

  • It is kept as single vararg string.
  • It also parsed onto tokens and stored in regular arguments collection.

Since don't know whether you need it at all, I've only activated GAS compatible rules.

Perhaps it would be dead Diff. But perhaps you will put your requests here, and I'll try to implement them ;-)

Here you can find:

  1. Patch himself.
  2. Test for darwin.

I also not sure about existing style. May be we refactor it a bit?

  1. Is it ok, we use structs everywhere? We could convert some of them into classes.

E.g.: MCAsmMacroParameter structure could be converted into:

class MCAsmMacroParameter {
  StringRef Name;
  MCAsmMacroArgument DefaultValue;
  bool Required;
  void setName(StringRef Name) { this->Name = Name; }
  StringRef getName() const { return Name; }

  void setDefaultValue(const MCAsmMacroArgument &V) { DefaultValue = V; }
  const MCAsmMacroArgument &getDefaultValue() const { return DefaultValue; }

  void setRequired(bool Required) { this->Required = Required; }
  bool getRequired() const { return Required; }

  MCAsmMacroParameter() : Required(false) {}

You'll get bigger constructions (P.getName() instead of P.Name). But.. well I don't know why, my intuition just hints me to prefer classes.
By now, I've kept structures.

  1. We could also use SmallVector instead of std::vector. Since collections are usually less than 8 items in size. I've kept std::vector in this diff.
  1. What do you think about "Substitution" term? In expandMacro we could use "ArrayRef<MCAsmMacroArgument> Substitutions" name then (instead of "ArrayRef<MCAsmMacroArgument> A").

So, if you agree, may be first we prepare refactor patch then?

Diff Detail

Event Timeline

dyatkovskiy updated this revision to Diff 8837.Apr 25 2014, 5:00 AM
dyatkovskiy retitled this revision from to Darwin vararg parameters support in assembler macros.
dyatkovskiy updated this object.
dyatkovskiy edited the test plan for this revision. (Show Details)
dyatkovskiy added reviewers: rafael, grosbach.
dyatkovskiy updated this object.
dyatkovskiy edited the test plan for this revision. (Show Details)
dyatkovskiy added a subscriber: Unknown Object (MLST).
grosbach edited edge metadata.Apr 25 2014, 10:43 AM

+Kevin, Tim

jannau added a subscriber: jannau.Jul 24 2014, 4:17 AM

I would like to see this get merged.

This is the only missing feature to assemble libav/FFmpeg's aarch64 handwritten assembly code for ios/darwin.

libav/FFmpeg's test suite passes on cyclone using clang/llvm with this patch as assembler/compiler.

Please add Tim Northover and Kevin Enderby as reviewers.


Hi Janne,
What exactly you'd like to merge? Did mean to use GNU style (non-Darwin) everywhere?

This comment was removed by dyatkovskiy.

Hi Stepan,

I would like to use vararg macro parameters when assembling code for Darwin. Libav/FFmpeg has an important set of handwritten assembler code for arm and aarch64 which uses gnu as style vararg arguments. I would like to use that code unmodified for ios.

After reading through AsmParser.cpp I'm think this patch is too complicated. There is no need to handle the darwin positional parameters $0, $1, ... and vararg at the same time. They exclude each other.

The positional parameters are only used when the macro definition has no parameters. A quick test with just removing the IsDarwin check in parseDirectiveMacro() seems to work fine.

jannau added a comment.Aug 5 2014, 7:40 AM

A different patch got committed with revision 214799.

espindola edited reviewers, added: espindola; removed: rafael.Mar 15 2018, 11:06 AM