dinfuehr / dora

Dora VM
MIT License
490 stars 31 forks source link

stdlib: implement first rounding methods for `Float32` and `Float64` #279

Closed soc closed 2 years ago

soc commented 2 years ago

See https://github.com/dinfuehr/dora/issues/140.

soc commented 2 years ago

@dinfuehr could you have a look?

dinfuehr commented 2 years ago

Why would we want to add e.g AVX encoding as well? I suppose advantages (if there are any) are miniscule at best but adds one more configuration to Dora.

soc commented 2 years ago

Hi @dinfuehr, this is due to https://github.com/dinfuehr/dora/issues/267. It's not that terrible, it's basically another line here:

   static ref HAS_ROUND: bool = is_x86_feature_detected!("sse4.1");
++ static ref HAS_AVX: bool = is_x86_feature_detected!("avx");
   static ref HAS_POPCNT: bool = is_x86_feature_detected!("popcnt");
   static ref HAS_LZCNT: bool =  is_x86_feature_detected!("lzcnt");
   static ref HAS_TZCNT: bool = is_x86_feature_detected!("bmi1");

I would have preferred only using AVX to start with, but requiring AVX is probably a few years too early (and wouldn't make sense with all the other checks we do here).

dinfuehr commented 2 years ago

We can add that later but let's not add all these optimizations which atm just complicate the code but do not bring any real speedup.

soc commented 2 years ago

Ok!

soc commented 2 years ago

@dinfuehr CI fails seem to be unrelated, I think one of your refactorings broke the linter.

dinfuehr commented 2 years ago

Thanks, should be fixed now.

soc commented 2 years ago

@dinfuehr all green!

soc commented 2 years ago

@dinfuehr I think this should be good to go, the CI failure seems to be unrelated.

soc commented 2 years ago

@dinfuehr could you have a look?

soc commented 2 years ago

@dinfuehr ping?