Fix Issue 18197 - Correct optimization for OPpair in x87 mode #8082
Conversation
|
I wonder why dlang bot hasn't picked this up. @LemonBoy spell Issue with a capital I in the commit? |
wilzbach
changed the title
|
Thanks for your pull request and interest in making D better, @LemonBoy! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
|
This first time when dlang-bot was called it got a 404 from the GH API: Not sure why though. It sometimes does run into the rate-limit, but then it gets a 403. |
src/dmd/backend/cgelem.c
Outdated
| !el_appears(e2, e1->EV.sp.Vsym) && | ||
| // Disable this rewrite if we're using x87 and the OPpair isn't made | ||
| // of FP values | ||
| (config.fpxmmregs || tyfloating(e2->E1->Ety) == tyfloating(e2->Ety)) |
There was a problem hiding this comment.
tyfloating doesn't return a bool:
inline unsigned tyfloating(tym_t ty) { return tytab[ty & 0xFF] & (TYFLreal | TYFLimaginary | TYFLcomplex); }
so the test should be tyfloating() != 0
src/dmd/backend/cgelem.c
Outdated
| !el_appears(e2, e1->EV.sp.Vsym) | ||
| !el_appears(e2, e1->EV.sp.Vsym) && | ||
| // Disable this rewrite if we're using x87 and the OPpair isn't made | ||
| // of FP values |
There was a problem hiding this comment.
I think you mean OPpair isn't made of mixed FP and non-FP values?
There was a problem hiding this comment.
I reworded the comment a bit, I hope it's clearer now.
Attempting to push a non-fp value into mST0 results in a ICE.
src/dmd/backend/cgelem.c
Outdated
| !el_appears(e2, e1->EV.sp.Vsym) | ||
| !el_appears(e2, e1->EV.sp.Vsym) && | ||
| // Disable this rewrite if we're using x87 and `e1` is a FP-value | ||
| // but `e2` is not |
src/dmd/backend/cgelem.c
Outdated
| // Disable this rewrite if we're using x87 and `e1` is a FP-value | ||
| // but `e2` is not | ||
| (config.fpxmmregs || | ||
| tyfloating(e2->E1->Ety) != 0 == tyfloating(e2->Ety) != 0) |
There was a problem hiding this comment.
Need parens here:
(tyfloating(e2->E1->Ety) != 0) == (tyfloating(e2->Ety) != 0)
Attempting to push a non-fp value into mST0 results in a ICE.
The patch assumes
OPpairis homogeneous, I hope that invariant holds (and should probably slip an assertion just to be sure).