Do not apply the flags of a conditional instruction in TransformConstDst - #516
Open
j-modernc-org wants to merge 1 commit into
Open
j-modernc-org wants to merge 1 commit into
j-modernc-org wants to merge 1 commit into
Conversation
A conditional instruction sets its flags only where its condition holds, so after it they are not known. TransformConstDst handed them to ApplyConditionAfter anyway: `if_e cmp f, #0 wz` folded as an unconditional compare turned the `if_e jmp` after it into a jmp, and `(12 != g) || f` came out 0 for a g of 0 (flexprop issue 118). Test/cexec06.c is the reproducer as a hardware test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the fix suggested in item 1 of totalspectrum/flexprop#118, with a test.
TransformConstDst folds an instruction whose operands are known, and hands the
flags it would set to ApplyConditionAfter without looking at
ir->cond. Aconditional instruction sets its flags only where its condition holds, so after it
they are not known. In the #118 reproducer,
if_e cmp f, #0 wzwas folded as anunconditional compare, the
if_e jmpafter it became ajmp, and(12 != g) || fcame out 0 for agof 0.The change returns early for a conditional instruction, as OptimizeCompares and the
constant moves in OptimizeMoves already do.
Test/cexec06.cis the reproducer as a hardware test. It prints0 1without thechange and
1 1with it.Checked, with flexspin built from this branch (master eb26396 plus this commit):
make test_offline: the same tests pass as on master.Test/runtests_p2.shon a P2-EDGE: 27 of 27 pass, cexec06 included.compile differently. All six run correctly on the board with the change, and one
of them fails without it.
Item 2 of #118 (the hang at nine nested calls) and item 3 (the question) are not
touched here.