Iterate in assembly order for jump threading
While reading through the jump threading implementation, I noticed something strange: ApplyForwarding iterates through the block list in reverse post-order, not in assembly order. Thus, the value prev_fallthru might not refer to the previous block in assembly order. Obviously it works fine this way or we would have noticed by now, but I think that this step would be a little easier to read and reason about if the iteration used assembly order instead. I've added a test case to demonstrate the difference when using assembly order: in a diamond where the right side starts with an empty deferred block, the current implementation would fail to replace that block with a nop. I doubt this case would have any real-world impact. Change-Id: I28abe2043434debb54896871d15c540ad52c6368 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3039261 Commit-Queue: Seth Brenith <seth.brenith@microsoft.com> Reviewed-by: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#76067}
This commit is contained in:
parent
80aed7e2cc
commit
fcc81d9a0c
@ -206,7 +206,7 @@ void JumpThreading::ApplyForwarding(Zone* local_zone,
|
||||
|
||||
// Skip empty blocks when the previous block doesn't fall through.
|
||||
bool prev_fallthru = true;
|
||||
for (auto const block : code->instruction_blocks()) {
|
||||
for (auto const block : code->ao_blocks()) {
|
||||
RpoNumber block_rpo = block->rpo_number();
|
||||
int block_num = block_rpo.ToInt();
|
||||
RpoNumber result_rpo = result[block_num];
|
||||
|
@ -736,7 +736,7 @@ TEST(Rewire1_deferred) {
|
||||
code.Defer();
|
||||
int j3 = code.Jump(3);
|
||||
// B3
|
||||
code.End();
|
||||
code.Return(0);
|
||||
|
||||
static int forward[] = {3, 3, 3, 3};
|
||||
ApplyForwarding(&code, kBlockCount, forward);
|
||||
@ -774,6 +774,29 @@ TEST(Rewire2_deferred) {
|
||||
CheckAssemblyOrder(&code, kBlockCount, assembly);
|
||||
}
|
||||
|
||||
TEST(Rewire_deferred_diamond) {
|
||||
constexpr size_t kBlockCount = 4;
|
||||
TestCode code(kBlockCount);
|
||||
|
||||
// B0
|
||||
int b1 = code.Branch(1, 2);
|
||||
// B1
|
||||
code.Fallthru(); // To B3
|
||||
// B2
|
||||
code.Defer();
|
||||
int j1 = code.Jump(3);
|
||||
// B3
|
||||
code.Return(0);
|
||||
|
||||
static int forward[] = {0, 3, 3, 3};
|
||||
VerifyForwarding(&code, kBlockCount, forward);
|
||||
ApplyForwarding(&code, kBlockCount, forward);
|
||||
CheckBranch(&code, b1, 3, 3);
|
||||
CheckNop(&code, j1);
|
||||
|
||||
static int assembly[] = {0, 1, 2, 1};
|
||||
CheckAssemblyOrder(&code, kBlockCount, assembly);
|
||||
}
|
||||
|
||||
TEST(Rewire_diamond) {
|
||||
constexpr size_t kBlockCount = 5;
|
||||
|
Loading…
Reference in New Issue
Block a user