search

pnggroup / libpng

LIBPNG: Portable Network Graphics support, official libpng repository
http://libpng.sf.net
Other
1.25k stars 611 forks source link

LoongArch LSX: Follow up on checking for compiler intrinsics inside ./configure #549

Closed ctruta closed 6 months ago

ctruta commented 6 months ago

In commit 14a348ddc839be9539a10e9085e76d3a6fdcb58b, we placed the following line in configure.ac

AC_MSG_CHECKING(whether to use LoongArch LSX intrinsics)

under the guard of the Loongson platform, because it only applies to that platform.

However, we only did this as an interim solution. A full solution, in line with the rest of the configure.ac patterns concering SIMD optimizations, is TODO.

ctruta commented 6 months ago

@XiWeiGu @jinboson Could you please help us with this? https://github.com/pnggroup/libpng/blob/14a348ddc839be9539a10e9085e76d3a6fdcb58b/configure.ac#L624

I'm asking because it is not clear to me whether that section is truly needed for your platform. (It's not needed for any other SIMD-enabled platform.)

I would also appreciate it if you could please test and confirm that everything is fine and nothing got broken after the integration of commit 14a348ddc839be9539a10e9085e76d3a6fdcb58b.

XiWeiGu commented 6 months ago

Thank you, I will proceed to confirm.

XiWeiGu commented 6 months ago

I would also appreciate it if you could please test and confirm that everything is fine and nothing got broken after the integration of commit 14a348d.

Everything is fine, thank you.

XiWeiGu commented 6 months ago

I'm asking because it is not clear to me whether that section is truly needed for your platform. (It's not needed for any other SIMD-enabled platform.)

The LSX vector instruction compiler detection on the LoongArch platform is somewhat unconventional compared to other platforms, as it checks whether the compiler supports LSX during configuration. The LoongArch toolchain does not have "__loongarch_sx" by default, so the "-mlsx" option needs to be explicitly added. If the conventional method used on other platforms is followed by passing CFLGAS='-mlsx' to enable LSX optimization, the '-mlsx' flag would apply globally instead of being restricted to files under the 'loongarch' directory. This would lead to automatic vectorization in files outside the 'loongarch' directory, potentially generating 'LSX' instructions. Thus, even with runtime detection enabled, issues could still arise.

ctruta commented 6 months ago

I understand. Thank you very much for the clarification.

ctruta commented 6 months ago

Considering the response by @XiWeiGu, I deleted the FIXME note in commit 890231026d265f20299f47f14e47838276b2cbb4, and I released libpng-1.6.43. Hopefully, everything LSX-related is in 100% working order.