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

COMMON: Use compiler intrinsics in SWAP_BYTES_16 when available #3962

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Jun 4, 2022

No description provided.

common/endian.h Outdated
@@ -106,6 +106,20 @@
return result;
}
}

// Test for GCC-compatible
#elif GCC_ATLEAST(4, 8) || defined(__clang__)
Copy link
Member

@lephilousophe lephilousophe Jun 4, 2022

Choose a reason for hiding this comment

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

I wonder if this one shouldn't come first (before PSP one) because it's cleaner than writing a piece of assembly code.

Copy link
Member

@lephilousophe lephilousophe Jun 4, 2022

Choose a reason for hiding this comment

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

Forget about it, I just saw how it's done for 32-bits.

Copy link
Member Author

@ccawley2011 ccawley2011 Jun 4, 2022

Choose a reason for hiding this comment

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

It might still be worth assessing how useful the MIPS routines are with modern compilers - according to the Compiler Explorer, GCC 5.4 for MIPS at least already generates wsbh/ror instructions when using the GCC builtins, and for MIPS64r2 the assembly code looks like it'll be slower than the builtins because it doesn't use dsbh/dshd for SWAP_BYTES_64. I'm not sure about the special Allegrex instructions, though.

Copy link
Member

@lephilousophe lephilousophe Jun 4, 2022

Choose a reason for hiding this comment

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

For Allegrex, I will do some test but it should be OK according to the code:
pspdev/gcc@1530c47#diff-dad60b351908e704b73fd0b46c72674f8fc9fff6fea1900b902d8b04fae3fb1fR5871

Copy link
Member

@lephilousophe lephilousophe Jun 4, 2022

Choose a reason for hiding this comment

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

I can confirm that with latest PSP toolchain (GCC 11.2.0 allegrex), __builtin_bswap32 generates wsbw and __builtin_bswap16 generates wsbh.

Copy link
Member Author

@ccawley2011 ccawley2011 Jun 19, 2022

Choose a reason for hiding this comment

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

OK, I've added another commit that removes the MIPS versions.

Copy link
Member

@lephilousophe lephilousophe left a comment

LGTM.

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