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

Jolt: Update to commit f094082aa, adding RISC-V, PPC64 and LoongArch support #100561

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akien-mga
Copy link
Member

Fixes #100557.

@akien-mga akien-mga force-pushed the jolt-disable-rv64-ppc64 branch from 9fa6da8 to d243302 Compare December 18, 2024 15:53
@akien-mga akien-mga changed the title Jolt: Disable module on unsupported RISC-V and PPC architectures Jolt: Disable module on unsupported RISC-V, PPC, and LoongArch architectures Dec 18, 2024
@hpvb
Copy link
Member

hpvb commented Dec 18, 2024

Jolt supports rv32 and rv64 as of jrouwe/JoltPhysics#1400 we should note that and when we update Jolt update this list of archs. It'd be nice to support these as our friendly neighborhood open source cpu arch :)

@akien-mga akien-mga force-pushed the jolt-disable-rv64-ppc64 branch from d243302 to 3861470 Compare December 19, 2024 13:49
@akien-mga
Copy link
Member Author

I backported JoltPhysics#1400 to add RISC-V support, so now this PR only disables the module for ppc32/ppc64/loongarch64.

@akien-mga akien-mga requested a review from mihe December 19, 2024 13:50
@akien-mga akien-mga changed the title Jolt: Disable module on unsupported RISC-V, PPC, and LoongArch architectures Jolt: Add RISC-V support, disable module on PPC and LoongArch architectures Dec 19, 2024
@mihe
Copy link
Contributor

mihe commented Dec 19, 2024

I backported JoltPhysics#1400 to add RISC-V support

Any reason why we wouldn't just update Jolt as a whole instead? The currently pinned version is largely just an arbitrary commit from Jolt's master branch, so isn't tied to any point release or anything like that.

@akien-mga
Copy link
Member Author

I backported JoltPhysics#1400 to add RISC-V support

Any reason why we wouldn't just update Jolt as a whole instead? The currently pinned version is largely just an arbitrary commit from Jolt's master branch, so isn't tied to any point release or anything like that.

Yeah we can do that too, I just haven't evaluated whether there's been any other upstream changes that might be a bit risky to use in production. Usually we prefer to stick to tagged releases but I see we're already on a dev commit so we might as well update further.

@akien-mga akien-mga force-pushed the jolt-disable-rv64-ppc64 branch from 3861470 to 0926be3 Compare December 20, 2024 10:34
@akien-mga akien-mga marked this pull request as draft December 20, 2024 10:35
@mihe
Copy link
Contributor

mihe commented Dec 20, 2024

Might be good to bump it by one more commit, to jrouwe/JoltPhysics@27559da, just in case anyone runs into problems with the MSVC warning mentioned in jrouwe/JoltPhysics#1402.

@akien-mga
Copy link
Member Author

Yeah, and I see jrouwe/JoltPhysics#1377 was reopened so I was thinking of waiting for that one to be merged too to sync.

@jrouwe
Copy link
Contributor

jrouwe commented Dec 20, 2024

Yes, please give me a couple of days. I'll get PPC and LoongArch support in there too. I'll ping this thread when it's done.

@akien-mga
Copy link
Member Author

Sounds good! No rush from our side, this is still a mostly theoretical ask and nothing urgent to solve.

@jrouwe
Copy link
Contributor

jrouwe commented Dec 20, 2024

PowerPC support has been merged, only Little Endian mode works atm. Big Endian compiles but fails the unit tests.

@jrouwe
Copy link
Contributor

jrouwe commented Dec 21, 2024

LoongArch support has also been added to Jolt.

I think this PR can now be updated to the latest version of Jolt and merged.

@akien-mga akien-mga force-pushed the jolt-disable-rv64-ppc64 branch from 0926be3 to 1c669d7 Compare December 21, 2024 14:13
@akien-mga akien-mga changed the title Jolt: Add RISC-V support, disable module on PPC and LoongArch architectures Jolt: Update to commit f094082aa, adding RISC-V, PPC64 and LoongArch support Dec 21, 2024
@akien-mga akien-mga marked this pull request as ready for review December 21, 2024 14:14
@akien-mga
Copy link
Member Author

akien-mga commented Dec 21, 2024

Amazing work, thanks a lot @jrouwe!

The PR was updated to simply update Jolt Physics to jrouwe/JoltPhysics@f094082.

We're still disabling the Jolt module for ppc32, which we apparently support compiling for otherwise... but that one is really a candidate for dropping support for, given that we're even wondering whether to keep x86_32...

@akien-mga akien-mga force-pushed the jolt-disable-rv64-ppc64 branch from 1c669d7 to 4727f07 Compare December 21, 2024 14:16
Copy link
Contributor

@mihe mihe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@jrouwe
Copy link
Contributor

jrouwe commented Dec 21, 2024

Was wondering why ppc32 was excluded, but I see now that it doesn't support 8 byte atomics, so this is indeed the best we can do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Godot (Jolt) cannot be cross-compiled on Linux for ppc64 and rv64
5 participants