Skip to content

Make translate-c use intermediate AST #7479

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

Merged
merged 36 commits into from
Feb 19, 2021
Merged

Make translate-c use intermediate AST #7479

merged 36 commits into from
Feb 19, 2021

Conversation

Vexu
Copy link
Member

@Vexu Vexu commented Dec 17, 2020

Will implement #6710

@Sirius902
Copy link

Will this by chance address #3453?

@andrewrk
Copy link
Member

Still working on this? Is there perhaps a way you can start the work in such a way that it doesn't have to be 100% completed before we can start utilizing it? That way we don't have a PR sitting around bit rotting.

@andrewrk andrewrk added the translate-c C to Zig source translation feature (@cImport) label Jan 12, 2021
@andrewrk
Copy link
Member

Closing this old draft - the code is still available and linked from the respective issue description for anyone to reference it. When we tackle this again let's try to do it incrementally so that the whole rework/refactor does not have to get done before we can start merging into master branch.

@andrewrk andrewrk closed this Jan 13, 2021
@Vexu Vexu deleted the translate-c-ast branch January 13, 2021 04:20
@Vexu Vexu restored the translate-c-ast branch January 13, 2021 04:21
@Vexu
Copy link
Member Author

Vexu commented Jan 18, 2021

This currently doesn't affect translate-c and could be merged as is for aomeone to continue, ci failure was probably unrelated.

@andrewrk
Copy link
Member

Ah I see, I will resurrect it and merge then!

@Vexu Vexu reopened this Jan 31, 2021
@Vexu Vexu changed the base branch from master to ast-memory-layout January 31, 2021 11:10
@andrewrk
Copy link
Member

This is going to make #7920 much easier to update translate-c because the only thing that will have change is the render() function.

@andrewrk
Copy link
Member

andrewrk commented Feb 1, 2021

Looks like the rebase got messed up

Vexu and others added 5 commits February 17, 2021 14:11
The previous iteration of translate-c used an incorrect block label
in the break statement for a translated C statement expression. This adds
a test to ensure the correct label is used in the new intermediate AST
version of translate-c.
@andrewrk
Copy link
Member

@Vexu whenever you feel this is ready to merge into ast-memory-layout, go for it

@ehaas
Copy link
Contributor

ehaas commented Feb 18, 2021

There is one error happening on this branch that isn't happening with the old translate-c - duplicate non-translatable macros are getting emitted twice as @compileError.

#define FOO 5=2
#define FOO 5=2

produces

pub const FOO = @compileError("unable to translate C expr: unexpected token .Equal"); // ddef.c:4:9
pub const FOO = @compileError("unable to translate C expr: unexpected token .Equal"); // ddef.c:5:9

Previously only one FOO was emitted. This happens to me on MacOS 11.2 when I #include <stdlib.h> due to duplicate definitions of __API_AVAILABLE_PLATFORM_macCatalyst.

@andrewrk
Copy link
Member

One thing that will be interesting to test out on this branch, once we get it compiling again, is this benchmark. Here it is on master branch, on my machine:

test.c

#include <windows.h>

(wipe global & local zig caches)

$ perf stat -e cache-misses -e instructions -e cycles ./zig translate-c test.c -target x86_64-windows-gnu -lc >windows_h.zig

 Performance counter stats for './zig translate-c test.c -target x86_64-windows-gnu -lc':

         6,632,603      cache-misses:u                                              
    40,151,754,610      instructions:u            #    1.39  insn per cycle         
    28,980,858,981      cycles:u                                                    

       6.669550308 seconds time elapsed

       6.539106000 seconds user
       0.132778000 seconds sys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants