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

remove inline from worker_dispatch_callout #294

Open
wants to merge 1 commit into
base: feat/headless
Choose a base branch
from

Conversation

syrel
Copy link
Contributor

@syrel syrel commented Jun 22, 2021

This is a funny one.

worker.c >> worker_dispatch_callout() is marked as inline and is called from workerPrimitives.c. However, Clang is smart enough to not create worker_dispatch_callout function at all, it literaly inlines it. However, it turns out that worker_dispatch_callout is called from the other object file which results in the linker error:

 lld-link : error : undefined symbol: worker_dispatch_callout [C:\Users\syrel\Desktop\gtoolkit-vm\build\PharoVMCore.vcxproj]

@guillep
Copy link
Member

guillep commented Jun 24, 2021

I'm ok with this change, but I'd like a second pair of eyes. @tesonep @estebanlm do you remember why there is an inline directive there?

@estebanlm
Copy link
Member

I don't remember :/
Is there a performance impact?
Maybe we can replace it with a macro, in that case.

hogoww added a commit to hogoww/opensmalltalk-vm that referenced this pull request Dec 23, 2021
hogoww added a commit to hogoww/opensmalltalk-vm that referenced this pull request Dec 23, 2021
…[ allocateOldSpaceChunkOfBytes: ] KILLED by 1/168 test cases.
hogoww added a commit to hogoww/opensmalltalk-vm that referenced this pull request Dec 29, 2021
hogoww added a commit to hogoww/opensmalltalk-vm that referenced this pull request Dec 29, 2021
…[ objectBytesForSlots: ] KILLED by 1/47 test cases.
@syrel
Copy link
Contributor Author

syrel commented Jan 23, 2022

Any news on this one? We are building the VM using Windows MSVC compiler. For this, we need to to address the inline word, otherwise the linker fails.

syrel added a commit to feenkcom/gtoolkit-vm that referenced this pull request Jan 23, 2022
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.

None yet

3 participants