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

Invalid Abs and Absf implementation, compiler optimization and external libraries #33

Open
FirefoxMetzger opened this issue Aug 30, 2016 · 7 comments

Comments

@FirefoxMetzger
Copy link

FirefoxMetzger commented Aug 30, 2016

constexpr inline float _MM_CALLCONV Absf(float x)
        {
            return (x < 0) ? x * -1 : x;
        }

Does not result in the same behavior as std::abs (or any other abs implementation I've checked to date). The reason for this is hidden in the IEEE 754 - 2008 Comparisons shall ignore the sign of zero (so +0 = -0). This means that the given implementation would return Absf(-0) ==> -0 as opposed to Absf(-0) ==> 0. So I assume is is a bug in the code? Same story for the double version.

Further the compiler cannot optimize the branch yealing poor performance (compared to just removing the sign bit). Which is what every implementation I've checked so far does.

Before I submit a pull request for this, is there any limitation on using an 'external' math library? This would should significantly improve performance if said library supports extended command sets like SSE4.2 or AVX2 (if present on the target architecture, which is quite common).

Usually I am not to picky about these optimizations, but imo the math operations (which are core operations for most algebra based programms) should be as efficient as humanly possible.

@Honeybunch
Copy link
Contributor

Good catch. This is definitely a bug and you're right; an external math library might be a good call. Last year we decided that for learning purposes we wanted to develop our own SIMD math library. However now with less people on the team, it's a bit harder to maintain something like that. I believe Matt is currently planning to replace HatchitMath with GLM. If you wanted to submit a PR that took care of that I think Matt would appreciate it but I'm not sure if he's started that process or not.

@FirefoxMetzger
Copy link
Author

I have not written any fixes yet due to inexperience regarding any library policy or full knowledge of any target platform. If it is only SIMD this project might already suffice. The downside is that it will probably involve some fiddling, when thinking about precompiling and deploying an actual game. On the other hand there is certain appeal in a math library using the GPU (which is something I wanted to toy around with since Vulkan went 1.0).

May I ask what platforms are targeted by this engine? By that I mean not just OS, but general architecture present on targeted devices (e.g. Vulkan support, SSE, ...).

@MattGuerrette
Copy link
Member

MattGuerrette commented Aug 30, 2016

Thanks for pointing out this error. Currently, Hatchit is being developed on Windows/Linux (Linux mainly atm) and is targeting Vulkan exclusively for its rendering engine. As far as SSE, I am planning on using GLM for all future development of Hatchit so avoid having to maintain our own math library. GLM supports some SIMD enhancements to its library, and I can only assume is being further developed to use SIMD extensions.

As far as target platforms. PC (Windows), and Linux are the only planned platforms. Is there anything more you would like to know?

@FirefoxMetzger
Copy link
Author

Sadly I am unfamiliar with GLM, but I will look into that. However, it looks similar to CUDA for Nvidia GPUs, which I used a while back.

You have a way better overview over the entire project and where the math library would be used. Is the potential bottleneck of exchanging data between CPU and GPU a concern?

@MattGuerrette
Copy link
Member

Yes, the transfer of data between CPU and GPU would be of concern. But the project is being architected with multi-threading at is core and with Vulkan allowing for significant reduction in CPU usage I'm hoping that the math library won't be a significant issue.

@FirefoxMetzger
Copy link
Author

I am probably wrong here, but wouldn't it be best to have two math libraries in that case? One for the GPU and one for the CPU? That way data doesn't need to be transfered forward and back just to do computation for the respective other lacking such library.

@Erbelding
Copy link
Member

Most of our bottlenecks for sending data to the gpu involve large buffers of non intrinsic data types i.e. vertex buffers, which don't require us to perform large cpu side computations. There may be potential for a float based library, but we haven't needed one yet.

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

No branches or pull requests

4 participants