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

rewriter: add hardcoded call gate post condition call with no args #470

Merged
merged 5 commits into from
Dec 18, 2024

Conversation

kkysen
Copy link
Contributor

@kkysen kkysen commented Nov 21, 2024

For now, the name of the post condition checking function is ${target_name}_post_condition, and it's hardcoded to only emit for dav1d_get_picture.

I'm not sure how to call the function with arguments, though. I was imagining the API be that the post condition function have the same arguments as the target function and a void return type.

It also takes 0 args for now. Later PRs will progressively add args.

@kkysen
Copy link
Contributor Author

kkysen commented Nov 21, 2024

For the args, @ayrtonm and I decided we should push the caller args onto the stack at the beginning (only the 6 args in registers for now) and then pop them back into the 6 registers to call the post condition function. So for now it'll be limited to up to 6 args. Once we have that working, we'll work on also passing the target function return value as an argument as well, and more than 6 args.

@ayrtonm
Copy link
Contributor

ayrtonm commented Nov 21, 2024

To add some other notes from our talk

  • we want to check arguments in post-conditions to validate out parameters
  • using the first 6 regs will clobber rdx which is fine if the return value is not 128 bits
  • as a next step for proper argument handling we may want to use one of the purely stack-based calling conventions for the check functions

@kkysen kkysen changed the title rewriter: add hardcoded call gate post condition call rewriter: add hardcoded call gate post condition call with no args Nov 22, 2024
@kkysen
Copy link
Contributor Author

kkysen commented Nov 22, 2024

@ayrtonm, can I merge this and split my stuff into smaller PRs? Next PR will do the 6 register args, which I'm working on now.

@ayrtonm
Copy link
Contributor

ayrtonm commented Nov 22, 2024

can I merge this and split my stuff into smaller PRs

Maybe just do a single PR where the commits can be reviewed individually

@kkysen kkysen force-pushed the kkysen/call-gate-post-condition-call branch from 8a8db26 to 94bbdb3 Compare November 25, 2024 14:23
Copy link
Contributor

@ayrtonm ayrtonm left a comment

Choose a reason for hiding this comment

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

LGTM after adding an option to enable/disable the post-validation codegen.

For now, the name of the post condition checking function is `${target_name}_post_condition`,
and it's hardcoded to only emit for `dav1d_get_picture`.
Even with no arguments, the stack is not 16-byte aligned,
so we `sub 8` to align it.
@kkysen kkysen force-pushed the kkysen/call-gate-post-condition-call branch from 94bbdb3 to 82a087d Compare December 16, 2024 09:28
…by default

We still want to be able to run the rewriter on `dav1d`
normally without post conditions while this is still a WIP.
Or on Arm, where IA2 still lacks support for post conditions.
@kkysen kkysen requested a review from ayrtonm December 16, 2024 10:26
@kkysen kkysen merged commit b331294 into main Dec 18, 2024
34 checks passed
@kkysen kkysen deleted the kkysen/call-gate-post-condition-call branch December 18, 2024 18:48
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

Successfully merging this pull request may close these issues.

2 participants