Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Enforce doubly-linked chained block #331

Merged
merged 2 commits into from
Jan 21, 2024
Merged

Conversation

qwe661234
Copy link
Collaborator

Previously, the chained block was a linear structure where previous block only pointed to the next block. Now, we have introduced additional information, allowing the next block to also reference the previous block. This modification enhance the deletion of replaced block.

Close: #329

Because we already have block chaining, there is no need for predicted
block.
@jserv jserv changed the title Doubly-link chained block Enforce doubly-linked chained block Jan 20, 2024
@jserv jserv requested a review from visitorckw January 20, 2024 15:14
src/utils.h Outdated Show resolved Hide resolved
src/utils.h Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/utils.h Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly mention Close #329 in the body of git commit messages.

src/riscv_private.h Outdated Show resolved Hide resolved
src/riscv_private.h Outdated Show resolved Hide resolved
src/riscv_private.h Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be aware of the terminology. It is referred as "doubly-linked chained block" instead of "doubly-link chained block."

@qwe661234 qwe661234 force-pushed the link_BB branch 2 times, most recently from d5d07ac to 467660a Compare January 20, 2024 15:53
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
if (block1 && block1->translatable)
if (ir->branch_taken && !set_has(set, ir->branch_taken->pc)) {
block_t *block1 = cache_get(rv->block_cache, ir->branch_taken->pc);
if (block1->translatable)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we check block1 before dereferencing?

Copy link
Collaborator Author

@qwe661234 qwe661234 Jan 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block1 must be existed because we correctly handle deleted block.

Previously, the chained block was a linear structure where previous
block only pointed to the next block. Now, we have introduced additional
information, allowing the next block to also reference the previous
block. This modification enhance the deletion of replaced block.

Close: sysprog21#329
@visitorckw
Copy link
Collaborator

LGTM, the patches indeed addressed the issue, thanks!

@jserv jserv merged commit 8b3a4af into sysprog21:master Jan 21, 2024
7 checks passed
@qwe661234 qwe661234 deleted the link_BB branch January 21, 2024 06:55
vestata pushed a commit to vestata/rv32emu that referenced this pull request Jan 24, 2025
Enforce doubly-linked chained block
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion fail when running jit-bf with ENABLE_JIT=1
3 participants