Fix bad logical-or implementation #175
Conversation
DrXiao
force-pushed
the
frontend/fix-log-or
branch
from
01b0e05 to
96d44f9
Compare
src/parser.c
Outdated
| * F | ||
| * ----------------------- test a | ||
| * | | | ||
| * | T | | ||
| * | F | | ||
| * ----------------------- test b | ||
| * | | | ||
| * | T | | ||
| * | F | | ||
| * ---------------------- test c | ||
| * | (*bb, 'then') | ||
| * | | | ||
| * | T | | ||
| * | | | ||
| * assign assign | ||
| * false true | ||
| * (shared_bb, ('then_next') | ||
| * 'else_bb') | | ||
| * | | | ||
| * | | | ||
| * | | | ||
| * ----------------------> ('end') |
There was a problem hiding this comment.
Can you refine the ASCII art? I didn't get the idea.
There was a problem hiding this comment.
I will think of better ASCII art to illustrate how it works here.
There was a problem hiding this comment.
I think @DrXiao want to express a short-circuit flow here for the logic a && b && c. If the evaluation (test) of a is false, the process will directly follow the left-hand path to the assign false corner at the bottom left, bypassing the evaluations of b and c.
Perhaps adding some arrows to indicate the direction of the flow could make the ASCII diagram clearer?
There was a problem hiding this comment.
/* bb1
* +------------+
* | teq a, #0 | False
* | bne bb2 | --------------->
* | b bb5 | |
* +------------+ |
* | |
* | True |
* bb2 | |
* +------------+ |
* | teq b, #0 | False |
* | bne bb3 | --------------->
* | b bb5 | |
* +------------+ |
* | |
* | True |
* bb3 | | bb5
* +------------+ False | +------------+
* | teq c, #0 | ------------------> | load 0 |
* | bne bb4 | | b bb6 |
* | b bb5 | +------------+
* +------------+ |
* | |
* | True |
* bb4 | |
* +-----------+ |
* | load 1 | |
* | b bb6 | |
* +-----------+ |
* | bb6 |
* | +--------+ |
* ---------> | | <----------
* | |
* +--------+
*
* bb3, bb4, bb5, and bb6 will be 'then', 'then_next', 'else_bb'
* and 'end', respectively.
* */Here is my current draft of the latest ASCII art. The purpose of adding the comment is to explain the use of the variables then, then_next, else_bb and end.
For finalizing the logical operation, it should use the mentioned variables to add some instructions to these basic blocks and properly connect them to each other.
There was a problem hiding this comment.
Here is my current draft of the latest ASCII art. The purpose of adding the comment is to explain the use of the variables
then,then_next,else_bbandend.
Can you rotate the above "figure" 90 degrees clockwise, so that it would look more compact?
There was a problem hiding this comment.
/* For example: a && b
*
*
* bb1 bb2 bb3
* +-----------+ +-----------+ +---------+
* | teq a, #0 | True | teq b, #0 | True | ldr 1 |
* | bne bb2 | ----> | bne bb3 | ----> | b bb5 |
* | b bb4 | | b bb4 | +---------+
* +-----------+ +-----------+ |
* | | |
* | False | False |
* | | |
* | +---------+ +--------+
* -------------> | ldr 0 | ------> | |
* | b bb5 | | |
* +---------+ +--------+
* bb4 bb5
*
* bb2, bb3, bb4, and bb5 will be 'then', 'then_next',
* 'else_bb' and 'end', respectively.
* */Does this updated figure look better?
Because the original example (a && b && c) may make the comment too long after rotating the figure, I decided to use the simplest case (a && b) to explain how it works here.
Because shecc originally generates bitwise-or instructions for logical-or operations, this commit improves both the frontend and backend so that it generates branch instructions for logical-or operations. The new implementation obeys short-circuit principle; that is, once the first operand is true, the second operand is not evaluated.
This commit adds more test cases to validate the correctness of logical-or operations after fixing the potential issues in the previous implementation.
DrXiao
force-pushed
the
frontend/fix-log-or
branch
from
96d44f9 to
5faf26e
Compare
|
Thank @DrXiao for contributing! |
3 participants
Because the current shecc still generates bitwise-or instructions for logical-or operations, the proposed changes improve both the frontend and backend so that it generates branch instructions for logical-or operations.
The new implementation obeys short-circuit principle; that is, once the first operand is true, the second operand is not evaluated. Also, the proposed changes add some test cases to validate the correctness.