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

Disassembler uses the same format for variables and registers #48692

Open
ghost opened this issue Mar 29, 2022 · 4 comments
Open

Disassembler uses the same format for variables and registers #48692

ghost opened this issue Mar 29, 2022 · 4 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug vm-technical-debt This label tries to capture all the technical debt that we have accumulated in the Dart VM

Comments

@ghost
Copy link

ghost commented Mar 29, 2022

Consider:

        ;; v18 <- BinaryDoubleOp:20(+, v50, v72) T{_Double}
0xd8    1e69294c               faddd v12, v10, v9

The disassembler uses v[0-9]+ both for the temporary variables and for the FPU registers on Arm64.
This leads to some confusion as e.g. v18 might refer to both a variable and a register, holding different values.

We should probably come up with some separate naming scheme for the variables to distinguish them from the registers

@ghost ghost added type-enhancement A request for a change that isn't a bug vm-technical-debt This label tries to capture all the technical debt that we have accumulated in the Dart VM labels Mar 29, 2022
@lrhn lrhn added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Mar 29, 2022
@mraleph
Copy link
Member

mraleph commented Mar 29, 2022

I am not sure where confusion can originate from. Disassembly can't refer to SSA names directly and IL instructions can't refer to register directly (only in location summaries) - so the namespaces are completely disjoint.

@ghost
Copy link
Author

ghost commented Mar 29, 2022

To me it's confusing that they get printed alongside each other.
Of course it's not the end of the world - with a little mental overhead I can easily tell which is which. But the less overhead the better.
It also makes it easier to for instance search back and forth to find the place where a given v is set or read if you don't get a number of false positives.

Are you reluctant to change these?

@mraleph
Copy link
Member

mraleph commented Mar 29, 2022

Are you reluctant to change these?

No, I am just saying that I don't think it's really a problem. It's like reading two different languages that contain the same word, e.g. "bad" in English and Danish refer to different things but nobody is really confused because you know which language you are reading at any given time. I will acknowledge though that search clashes might be an issue (but again, you can easily filter things out with regexps - because IL comments start with \s*;;).

We could replace v with % or something similarly neutral (e.g. $).

@rmacnak-google
Copy link
Contributor

For RISC-V, there's another similar overlap between registers and temps in unoptimized code called t0 etc.

        ;; t0 <- Constant(#Closure: (dynamic) => void from Function '_print@15225868': static.)
0x7f0e94700292    0207b283               ld t0, 32(pp)   Closure: (dynamic) => void from Function '_print@15225868': static.
0x7f0e94700296        1161               addi sp, sp, -8
0x7f0e94700298        e016               sd t0, 0(sp)

It's mildly annoying when searching through disassembly to get hits from the wrong kind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug vm-technical-debt This label tries to capture all the technical debt that we have accumulated in the Dart VM
Projects
None yet
Development

No branches or pull requests

3 participants