search

supranational / blst

Multilingual BLS12-381 signature library
Apache License 2.0
454 stars 171 forks source link

Unoptimized sha256 funcs always do runtime detection #183

Closed jtraglia closed 11 months ago

jtraglia commented 11 months ago

Hi there. I noticed that the two routines for SHA are missing __BLST_PORTABLE__ checks:

https://github.com/supranational/blst/blob/3dd0f804b1819e5d03fb22ca2e6fac105932043a/src/asm/sha256-armv8.pl#L346-L350

And here in the x86 function: https://github.com/supranational/blst/blob/3dd0f804b1819e5d03fb22ca2e6fac105932043a/src/asm/sha256-x86_64.pl#L346-L350

So if the unoptimized sha256_block_data_order function is called, runtime detection will occur here without the __BLST_PORTABLE__ build flag. This will happen if __SHA__ and __ARM_FEATURE_CRYPTO are not defined. I only noticed when comparing benchmarks and saw the same performance with & without the portable flag.

Just to be clear, this isn't a problem for me but I don't think it's what you want.

dot-asm commented 11 months ago

Just to be clear, this isn't a problem for me but I don't think it's what you want.

No, it was intentional. The rationale is that since portable switches to adx, why wouldn't it switch to shaext.

jtraglia commented 11 months ago

This is for non-portable builds though. By default it will do a runtime switch for sha functions, right?

dot-asm commented 11 months ago

Ah! I see your point. Still intentional:-) It would be weird to "punish" non-portable build in denying the switch.

In case you sense inconsistency in respect to being more rigid about ADX. Well, with ADX it's insufficient to simply pass some test vectors (based even on experience:-), in other words the verification bar is much higher.

jtraglia commented 11 months ago

Okay cool, that makes sense to me. Thanks 😄