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.

Sign up for GitHub

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

Jump to bottom

AArchXX clean calls handle far too few use cases, blocking tool development #2210

Open
derekbruening opened this issue Feb 22, 2017 · 7 comments
Open

AArchXX clean calls handle far too few use cases, blocking tool development #2210

derekbruening opened this issue Feb 22, 2017 · 7 comments
Labels
Bug-ToolFail Hotlist-Release OpSys-AArch64 OpSys-ARM Priority-High

Comments

@derekbruening
Copy link
Contributor

The AArchXX implementation of insert_parameter_preparation() is missing some key functionality:

/* FIXME i#1551, i#1569: we only implement naive parameter preparation,
 * where args are all regs or immeds and do not conflict with param regs.
 */

This is a big enough missing port feature to call for a separate issue. This is blocking tool development on AArchXX, even simple samples: #2174

Xref original issues #1551, #1569
@egrimley egrimley self-assigned this Mar 1, 2017
egrimley pushed a commit that referenced this issue Mar 2, 2017
@egrimley-arm
i#2210 clean call: Implement parameter preparation on ARM and AArch64.
0efa381 
The implementation should efficiently handle any (reasonable) number
of arguments that are immediate integers or full-size registers.
Test cases are added in cleancallparams.dll.c.

Fixes #2210
@egrimley egrimley mentioned this issue Mar 2, 2017
Merged
@derekbruening derekbruening reopened this Mar 5, 2017
@derekbruening
Copy link
Contributor Author

This shouldn't have been closed as handling memory operands as arguments is not yet finished.

@egrimley
Copy link
Contributor

egrimley commented Mar 5, 2017

(Why did it get closed? A bug in GitHub?)

@egrimley egrimley removed their assignment Mar 5, 2017
@egrimley
Copy link
Contributor

egrimley commented Mar 5, 2017

Memory operands will need a somewhat different approach as there will be cases (even if they occur very rarely in practice) in which it is necessary to spill and restore registers during the marshalling: imagine (31 * 31) arguments of the form [Xn, Xm], for example.

We should start by specifying exactly which types of memory operands should be accepted.

@derekbruening
Copy link
Contributor Author

(Why did it get closed? A bug in GitHub?)

Your commit message closed it by saying "Fixes #2210"

@derekbruening
Copy link
Contributor Author

It seems fine to disallow hard-to-handle memory operands if they're unlikely to be used much by clients.

@egrimley
Copy link
Contributor

egrimley commented Mar 5, 2017

Really? I can't see those words on this page: e06883c

@derekbruening
Copy link
Contributor Author

(Why did it get closed? A bug in GitHub?)

Your commit message closed it by saying "Fixes #2210"

Really? I can't see those words on this page: e06883c

Hmm, right, it's not there in the final squash-and-merge commit, only in the first one in the pull request. Maybe worth filing a github bug.

@egrimley egrimley mentioned this issue Dec 6, 2017
Open
@derekbruening derekbruening mentioned this issue Jun 14, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug-ToolFail Hotlist-Release OpSys-AArch64 OpSys-ARM Priority-High
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@derekbruening @egrimley