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

Attempt fix and enable for Ubuntu 22 #6975

Closed
wants to merge 3 commits into from

Conversation

rhuanjl
Copy link
Collaborator

@rhuanjl rhuanjl commented Apr 16, 2024

Change from #6932 + enable CI for Ubuntu 22

@ppenzin
Copy link
Member

ppenzin commented Apr 16, 2024

I've played around with this, not sure if there is just one place where the alignment is needed, but hopefully we can figure this out.

@ShortDevelopment
Copy link
Contributor

#6847 (comment)

@ppenzin
Copy link
Member

ppenzin commented Apr 17, 2024

@rhuanjl I've added @ShortDevelopment's fix to the PR, but since it is your branch, feel free to remove/rework/change in anyway you want.

@ppenzin
Copy link
Member

ppenzin commented Apr 17, 2024

And I had to remove that fix, the issue is that MSVC doesn't like noinline attribute, so it has to be an ifdef or something like that - we can revisit. The code would work on Ubuntu and not necessary on Windows.

@ppenzin ppenzin mentioned this pull request Apr 17, 2024
@ppenzin
Copy link
Member

ppenzin commented Apr 17, 2024

I've updated the workaround to hopefully work on MSVC, trying it out in #6977, though not sure it is the optimal solution in the long run.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Apr 17, 2024

Planning to merge #6932 then we can follow up with a working version of this.

@rhuanjl rhuanjl closed this Apr 17, 2024
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.

3 participants