Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add gnu inline asm syntax #140

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

benjaminBrownlee
Copy link
Contributor

Add the GNU inline assembly syntax. Let me know if you think the names of nodes or the tree structure should be different. The structure is a bit more lax than the specification, but I think it makes sense for simplicity.

closes #139

@amaanq
Copy link
Member

amaanq commented Jul 11, 2023

I have something similar in my Objective-C grammar, is it more or less the same? https://github.com/amaanq/tree-sitter-objc/blob/7938eb5e135574095d8baf2c758e1f69d8cccafa/grammar.js#L372

I also have one for ms's version, __asm in the ms_asm_block

@benjaminBrownlee
Copy link
Contributor Author

I think they are trying to do the same thing, but there are some issues with the objective-c version:

  • input operands can be expressions; it is common to have casts in that context
  • goto and inline are also qualifiers, and you can have multiple
  • the clobber list should be a sequence of strings, not identifiers
  • missing goto label list
  • missing named registers

@amaanq
Copy link
Member

amaanq commented Jul 12, 2023

I added the optional identifiers after the strings because of this
https://github.com/apple-oss-distributions/objc4/blob/c3f002513d195ef564f3c7e9496c2606360e144a/test/release-workaround.m#L26

Is this valid then only in Objective-C?

The other changes are good, I can then easily extend off of that from C which would be nice 🙂

@benjaminBrownlee
Copy link
Contributor Author

We are talking about different parameters. The asm function has one required parameter (a string) and then up to four optional parameters separated by colons, which are the output operands, input operands, clobber list, and goto labels. Your link shows an example with an output operand but I was talking about the clobber list.

The last line of my test case uses a clobber list (with just "r2").

asm volatile (
"mov r0, %0\n"
"mov r1, %[y]\n"
"add r2, r0, r1\n"
"mov %1, r2\n"
: "r" (z)
: "=r" (x),
[y] "=r" ((uintptr_t) y)
: "r2");

From what I can tell, the asm syntax for obj-c, c and c++ is the same, but may need to be updated to support other features (ie raw string literals).

@benjaminBrownlee
Copy link
Contributor Author

For reference, the GNU specification for extended inline assmebly.

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

@amaanq
Copy link
Member

amaanq commented Jul 12, 2023

We are talking about different parameters. The asm function has one required parameter (a string) and then up to four optional parameters separated by colons, which are the output operands, input operands, clobber list, and goto labels. Your link shows an example with an output operand but I was talking about the clobber list.

The last line of my test case uses a clobber list (with just "r2").

asm volatile (
"mov r0, %0\n"
"mov r1, %[y]\n"
"add r2, r0, r1\n"
"mov %1, r2\n"
: "r" (z)
: "=r" (x),
[y] "=r" ((uintptr_t) y)
: "r2");

From what I can tell, the asm syntax for obj-c, c and c++ is the same, but may need to be updated to support other features (ie raw string literals).

Oh yeah, sorry about that! I appreciate the gnu reference - but I believe other compilers support it as well - can we rename the rules to drop the gnu prefix?

@benjaminBrownlee
Copy link
Contributor Author

While the main compilers (clang, gcc) all support this syntax, it is a GNU extension and not a language standard. I guess I chose the gnu_ prefix to distinguish it from other assembly syntax which might be added, such as MSVC or ARMCC 4/5, which I would probably prefix with a msvc_ and armcc_ respectively.

You want to drop the prefix altogether and figure out the other syntax names later? Or perhaps there is a prefix you like better?

@amaanq
Copy link
Member

amaanq commented Jul 13, 2023

If it's a gnu extension then no worries (I wasn't sure about that), this looks good!

@amaanq amaanq merged commit 693d298 into tree-sitter:master Jul 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support gnu extended asm syntax
2 participants