loongson-community / discussions

Cross-community issue tracker & discussions / 跨社区工单追踪 & 讨论场所
11 stars 0 forks source link

[PCRE2] Need to tell SLJIT about FP-to-int conversion characteristics #36

Closed xen0n closed 3 months ago

xen0n commented 10 months ago

When compiling libpcre2 10.43-RC1 I got this:

In file included from /var/tmp/portage/dev-libs/libpcre2-10.43_rc1/work/pcre2-10.43-RC1/src/sljit/sljitLir.h:82,
                 from /var/tmp/portage/dev-libs/libpcre2-10.43_rc1/work/pcre2-10.43-RC1/src/sljit/sljitLir.c:27,
                 from /var/tmp/portage/dev-libs/libpcre2-10.43_rc1/work/pcre2-10.43-RC1/src/pcre2_jit_compile.c:79:
/var/tmp/portage/dev-libs/libpcre2-10.43_rc1/work/pcre2-10.43-RC1/src/sljit/sljitConfigInternal.h:403:2: error: #error "Result for float to integer conversion is not defined"
  403 | #error "Result for float to integer conversion is not defined"
      |  ^~~~~
make[1]: *** [Makefile:2503: src/libpcre2_8_la-pcre2_jit_compile.lo] Error 1

https://github.com/zherczeg/sljit/blob/cbde0bea60df961234502e7cf45b1c8f283470fd/sljit_src/sljitConfigInternal.h#L86-L94

https://github.com/zherczeg/sljit/blob/cbde0bea60df961234502e7cf45b1c8f283470fd/sljit_src/sljitConfigInternal.h#L378-L404

We need to do some little experiments (doing conversions the SLJIT way with the inputs described, and recording the results) in order to be able to tell SLJIT how LoongArch's FP-to-int conversions behave.

xen0n commented 10 months ago

cc @lrzlin -- you might be interested.

lrzlin commented 10 months ago

cc @lrzlin -- you might be interested.

Patch: https://github.com/zherczeg/sljit/pull/226

xen0n commented 10 months ago

Build and tests pass after applying the patch, and I can see it working through consumers such as rg ;-)

image

It would be nice to have this pushed into libpcre2's release branch so we would have a working libpcre2-10.43 out of the box.

xen0n commented 10 months ago

https://github.com/gentoo/gentoo/pull/34729

lrzlin commented 10 months ago

It would be nice to have this pushed into libpcre2's release branch so we would have a working libpcre2-10.43 out of the box.

sljit author & pcre2 maintainer zherczeg would merge the lastest sljit into pcre2 almost every month then, also I'll working on sljit LSX/LASX support as soon as possible.

xry111 commented 10 months ago

cc @lrzlin -- you might be interested.

Patch: zherczeg/sljit#226

But this patch seems relying on the implicit behavior documented nowhere. The instruction manual only says the ftint instructions satisfy IEEE 754-2008 where these results are unspecified.

We cannot just rely on the Hyrum rule here because the user base of LoongArch is not large enough. We should either avoid relying on the undocumented behavior, or tell the hardware engineers to add this into the specification and keep it in future products.

lrzlin commented 10 months ago

cc @lrzlin -- you might be interested.

Patch: zherczeg/sljit#226

But this patch seems relying on the implicit behavior documented nowhere. The instruction manual only says the ftint instructions satisfy IEEE 754-2008 where these results are unspecified.

We cannot just rely on the Hyrum rule here because the user base of LoongArch is not large enough. We should either avoid relying on the undocumented behavior, or tell the hardware engineers to add this into the specification and keep it in future products.

Yes, this patch is purely rely on actual implementation, and I think LoongArch should following RISC-V practice, add the float-to-integer conversions and behavior for invalid inputs to the specification.

BTW, there is a OpenSSL patch for ChaCha20 asm's performance issue, could you give it a look? Thanks a lot. https://github.com/openssl/openssl/pull/23301

xry111 commented 10 months ago

BTW, there is a OpenSSL patch for ChaCha20 asm's performance issue, could you give it a look? Thanks a lot. openssl/openssl#23301

Already done. I'm too stupid :(.

xry111 commented 9 months ago

It would be nice to have this pushed into libpcre2's release branch so we would have a working libpcre2-10.43 out of the box.

sljit author & pcre2 maintainer zherczeg would merge the lastest sljit into pcre2 almost every month then, also I'll working on sljit LSX/LASX support as soon as possible.

Unfortunately this is not included in PCRE2-10.43.

lrzlin commented 4 months ago

Unfortunately this is not included in PCRE2-10.43.

At lease it could catch up in PCRE2-10.44, which could be the last release recently. https://github.com/PCRE2Project/pcre2/releases/tag/pcre2-10.44

xen0n commented 3 months ago

Confirmed fixed in PCRE2 10.44. Thanks for all the work!