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

Support far global variable accesses in ARM #921

Merged
merged 1 commit into from
Nov 23, 2024

Conversation

sarranz
Copy link
Collaborator

@sarranz sarranz commented Oct 2, 2024

Temporary patch for #619. I assume that ADR instructions are only generated for accesses to the data section, which seems to be true. I don't think it's a robust solution, but I don't see how to do it otherwise. Also, I believe we can't tell whether we can perform the relative access or not, so as to generate two instructions only when needed. The other options discussed in the issue didn't work, and I can't explain why.

@bgregoir
Copy link
Contributor

bgregoir commented Oct 8, 2024

I don't know what to think about this.
In one side it maybe is better than nothing.
On the other this is not certified.
Maybe you should merge this and then do the proper patch.
Any opinion on that ?

@sarranz
Copy link
Collaborator Author

sarranz commented Oct 8, 2024

I did this because I thought we couldn't verify much about this, but you're right (as usual :)). I guess what you have in mind is to define the MOV lower and upper as extra ops? If so, I can do it, no problem. But I can't assure when, so I leave the question about merging or waiting to you.

@bgregoir
Copy link
Contributor

bgregoir commented Nov 5, 2024

Hi Santiago,
have you been able to progress on this ?
B

@sarranz
Copy link
Collaborator Author

sarranz commented Nov 5, 2024

No. I'm at a loss because I can reproduce certain behaviors only in some assemblers, in particular this method with upper16 and lower16. I need to do a deep dive in the documentation next week when I have some time.

@sarranz
Copy link
Collaborator Author

sarranz commented Nov 15, 2024

Ok, JC has helped me out. Apparently the disassembler was showing me incomplete information and this approach is fine. I will cleanup the pretty printer for the moment like we discussed and we can plan a cleaner patch next week.

@sarranz sarranz force-pushed the arm-far-global-adr1 branch from ab88800 to 66de436 Compare November 15, 2024 18:40
@sarranz sarranz force-pushed the arm-far-global-adr1 branch from 66de436 to 6e1b404 Compare November 23, 2024 11:34
@sarranz
Copy link
Collaborator Author

sarranz commented Nov 23, 2024

Benjamin suggested we check that the argument is Addr (Arip _), so now the assumption about issuing ADR only for global accesses is not necessary.

@bgregoir bgregoir merged commit 3c5a18b into jasmin-lang:main Nov 23, 2024
@vbgl vbgl added this to the 2024.07.3 milestone Nov 29, 2024
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.

3 participants