Fix codegen branch for cond ? ~0 : 0#14945
Conversation
|
Thanks for your pull request and interest in making D better, @dkorpel! 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 referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "stable + dmd#14945" |
|
The testcase was added in the previous PR. |
| cdb.gen2(0xF6 + (opcode & 1),grex | modregrmx(3,2,reg)); // NOT reg | ||
| if (I64 && sz2 == REGSIZE) | ||
| if (I64 && sz2 == 1 && reg >= 4) | ||
| code_orrex(cdb.last(), REX_W); |
There was a problem hiding this comment.
No. Leave 2417 and 2418 intact. Insert the following after 2418:
if (I64 && sz2 == 1 && reg >= 4)
code_orrex(cdb.last(), REX);
There was a problem hiding this comment.
This pattern is used in the surrounding code. Whenever you see an odd 'H' register in the codegen output, this is the fix.
There was a problem hiding this comment.
Thanks. It's a bit concerning the test suite isn't red because of this.
compiler/src/dmd/backend/cod2.d
Outdated
| { | ||
| cdb.gen2(0xF6 + (opcode & 1),grex | modregrmx(3,2,reg)); // NOT reg | ||
| if (I64 && sz2 == REGSIZE) | ||
| code_orrex(cdb.last(), REGSIZE); |
There was a problem hiding this comment.
code_orrex(cdb.last(), REX_W);
compiler/src/dmd/backend/cod2.d
Outdated
| cdb.gen2(0xF6 + (opcode & 1),grex | modregrmx(3,2,reg)); // NOT reg | ||
| if (I64 && sz2 == REGSIZE) | ||
| code_orrex(cdb.last(), REGSIZE); | ||
| if (I64 && sz2 == 1 && reg >= 4) |
There was a problem hiding this comment.
Next line is:
code_orrex(cdb.last(), REX);
|
@dkorpel what is happening with this? |
|
Ironically, it was all green when it had the wrong code twice, and now when I finally got your suggestion right, the test suite spuriously fails. |
|
I've force pushed multiple times now, but some tests are still stuck |
5 participants
#14919 (comment)