Skip to content

Conversation

aaronfranke
Copy link
Member

Implements godotengine/godot-proposals#3374 on master. Only Clang Debug builds work on RISC-V right now.

Copy link
Contributor

@AaronRecord AaronRecord Oct 7, 2021

Choose a reason for hiding this comment

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

Why isn't this

Suggested change
supported_arch = env["bits"] == "64" and env["arch"] != "arm64" and not env["arch"].startswith("rv")
supported_arch = env["bits"] == "64" and env["arch"] != "arm64" and env["arch"] != "rv64"

?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the future we might want to allow specifying a custom -march string, like "rv64gcv". There's not a lot of use for this at the moment so I didn't bother supporting it in the SConstruct file, but the startswith check is a simple improvement over checking string inequality that will work for that case. Plus it's possible that someone would want to make a 32-bit or 128-bit version, and startswith also works for that case with no downsides.

Copy link
Member

Choose a reason for hiding this comment

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

Nitpick, but since we import platform already, I'd prefer simply using platform.machine() instead of re-importing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, this was just a blind copy from SConstruct so I missed that we already import platform.

Supports RV64GC (RISC-V 64-bit with general-purpose and compressed-instruction extensions)
@akien-mga akien-mga merged commit 9282b0d into godotengine:master Oct 22, 2021
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the riscv branch October 22, 2021 19:33
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.

3 participants