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

Fix Int64Fixture.RemI8I8 and Int64Fixture.DivI8I8 unit test case failures #54

Closed
tgiphil opened this issue May 27, 2014 · 11 comments · Fixed by #87
Closed

Fix Int64Fixture.RemI8I8 and Int64Fixture.DivI8I8 unit test case failures #54

tgiphil opened this issue May 27, 2014 · 11 comments · Fixed by #87

Comments

@tgiphil
Copy link
Member

tgiphil commented May 27, 2014

No description provided.

@tgiphil tgiphil added this to the 1.2.7 Bug Fixes and Optimizations milestone May 27, 2014
@tgiphil tgiphil added the Bug label May 27, 2014
@charsleysa
Copy link
Member

Has it been considered to use the FPU for 64bit arithmetics on x86?

@tgiphil
Copy link
Member Author

tgiphil commented Aug 10, 2014

No. We will use the SSE extensions.

The old FPU is stacked based and too difficult to adapt with use with a register allocator.

@charsleysa
Copy link
Member

What about having the algorithm as a static method in the Runtime and calling that algorithm? It would be easier to implement and the only downside would be having a call. I think that would be a fair compromise in order to implement 64bit arithmetics in 32bit, maybe we can even improve the compiler by inlining that call.

@tgiphil
Copy link
Member Author

tgiphil commented Aug 11, 2014

Yes; I have a branch of MOSA called 64BitIn32Bit that is working on just that. This would be a portable implementation divide and remainder for other 32bit architectures as well. Unfortunately, it's more buggy than what we have at the moment.

BTW. x86 FPU does floating point, not integer, arithmetic

@charsleysa
Copy link
Member

@tgiphil I took a quick run at this and I have managed to get it working for 64bit integer division.
It was based off your 64bitIn32bit branch but the TinySimulator was broken so I had to implement a couple of tests in TestWorld.
https://github.com/charsleysa/MOSA-Project/tree/64bitIn32bit

The code is basically taken from here and re-jigged to suit our needs:
http://www.informit.com/guides/content.aspx?g=dotnet&seqNum=642

@tgiphil
Copy link
Member Author

tgiphil commented Aug 14, 2014

Great! I'll take a look at this tonight.

Note: I started to work on exceptions.

@tgiphil
Copy link
Member Author

tgiphil commented Aug 21, 2014

@charsleysa - I was not able to confirm the implementation as working. The additional tests in TestWorld pass before the new 64bit integer division is implementation. In addition, the test suite had more failed test after the new implementation as well.

You indicated TinySimulator was broken, can you be more specific?

@charsleysa
Copy link
Member

@tgiphil you had made breaking changes in TinySimulator with the 32bit memory storage, that made it impossible to test using unit tests.

So I just found out (well this is embarrassing) that the C# compiler was optimizing the tests I had placed in TestWorld and no actual division takes place since it's pre-calculated. :(

@charsleysa
Copy link
Member

@tgiphil I found a bug in your 32bit memory storage code for TinySimulator that makes Unit Tests fail.
Pull Request coming soon.

@charsleysa
Copy link
Member

@tgiphil #86 fixes the bug.

Also I have found the problem in my division code and will soon (when I have some more spare time) attempt to re-implement it since I have now fixed the TinySimulator bug.

@charsleysa
Copy link
Member

@tgiphil after a few days of trial and error (I'm not great at 64bit signed arithmetics) I managed to create code that works and passes the Unit Tests.

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

Successfully merging a pull request may close this issue.

2 participants