Shopify / yjit

Optimizing JIT compiler built inside CRuby
650 stars 21 forks source link

Put YJIT into a single compilation unit #270

Closed XrXr closed 2 years ago

XrXr commented 2 years ago

For upstreaming, we want functions we export either prefixed with "rb_" or made static. Historically we haven't been following this rule, so we were "leaking" a lot of symbols as make leak-globals would tell us.

This change unifies everything YJIT into a single compilation unit, yjit.o, and makes everything unprefixed static to pass make leak-globals. This manual "unified build" setup is similar to that of vm.o.

Having everything in one compilation unit allows static functions to be visible across YJIT files and removes the need for declarations in headers in some cases. Unnecessary declarations were removed.

Other changes of note:

Fixes #266

maximecb commented 2 years ago

Can we just use the same mechanism in the asm tests (compile that in to a single unit) and not need YJIT_ASM_BUNDLED ?

XrXr commented 2 years ago

switched to MJIT_SYMBOL_EXPORT_BEGIN which indicates stuff as being off limits for native extensions

Since we don't support calling YJIT code from MJIT, it might be possible to make it so that we don't export these at all. Off topic here though.

XrXr commented 2 years ago

Can we just use the same mechanism in the asm tests (compile that in to a single unit) and not need YJIT_ASM_BUNDLED?

Sounds good. Does that mean it's time to stop trying to use CRuby stuff in the assembler?

maximecb commented 2 years ago

Sounds good. Does that mean it's time to stop trying to use CRuby stuff in the assembler?

I suppose we can.

XrXr commented 2 years ago

Sorry, I meant "stop trying to keep the assembler kind of separate from the rest of CRuby". Anyways, it shouldn't be using much of CRuby's facilities anyways except for maybe assertions.

maximecb commented 2 years ago

Yes we can do that if we want to 👍

XrXr commented 2 years ago

The CI failure should be fixed by #271