Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ARM] Incorrect dependency graph for store dependent on post-indexed load #90

Closed
dgazzoni opened this issue Feb 19, 2023 · 1 comment
Closed

Comments

@dgazzoni
Copy link
Contributor

Consider this non-sensical, but minimal, sequence of instructions:

    ldr x0, [x1], #8
    str x1, [sp]

This is the dependency graph generated by OSACA:

image

I would expect a dependency from ldr to str due to the update of x1.

Note that replacing the code above by an explicit addition of 8 to x1 generates a dependency from the addition to the store, just as I would expect:

    ldr x0, [x1]
    add x1, x1, #8
    str x1, [sp]

Dependency graph:

image

Unless I misunderstood how these dependency graphs are supposed to work, I believe that's a bug.

@dgazzoni
Copy link
Contributor Author

Analyzing the code, I'd like to draw attention to this check:

                    if self.is_read(dst.register, instr_form) and not (
                        dst.get("pre_indexed", False) or dst.get("post_indexed", False)
                    ):
                        yield instr_form, []

I'm not sure if I understand the need for the checks on "pre_indexed" and "post_indexed" there. I tried removing these conditions, i.e. rewriting the code to this:

                    if self.is_read(dst.register, instr_form):
                        yield instr_form, []

This appears to fix the issue. This is the new dependency graph for the first code snippet in the initial bug report (just post-indexed ldr followed by str):

image

However, given that I don't understand why the condition is there, I have no idea if this breaks something else.

@JanLJL JanLJL closed this as completed in 7e6eb7c Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant