Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

hca: Encoding field 16 bit even for 32 bit push instructions #95

Open
kjetilos opened this issue Jul 12, 2021 · 4 comments
Open

hca: Encoding field 16 bit even for 32 bit push instructions #95

kjetilos opened this issue Jul 12, 2021 · 4 comments

Comments

@kjetilos
Copy link

Hi,

When analyzing some code with the hca script I see the following output.

{'PC': '8fe29930', 'Encoding': '8436', 'WoE': 32, 'Instruction': 'push', 'Category': 'alu', 'Destination': ('s0',), 'Source': ('ra', 's0-s5')}
{'PC': '8fe29e10', 'Encoding': '13612823', 'WoE': 32, 'Instruction': 'push', 'Category': 'store', 'Source': ('ra', 's0-s6'), 'Immediate': '304'}
{'PC': '8fe29f84', 'Encoding': '8432', 'WoE': 32, 'Instruction': 'push', 'Category': 'alu', 'Destination': ('s0',), 'Source': ('ra', 's0-s8')}
{'PC': '8fe2a0ce', 'Encoding': '15912a23', 'WoE': 32, 'Instruction': 'push', 'Category': 'store', 'Source': ('ra', 's0-s9'), 'Immediate': '340'}
{'PC': '8fe2a2aa', 'Encoding': 'dbe6', 'WoE': 32, 'Instruction': 'push', 'Category': 'store', 'Source': ('ra', 's0-s9'), 'Immediate': '244'}
{'PC': '8fe2a41e', 'Encoding': '15912a23', 'WoE': 32, 'Instruction': 'push', 'Category': 'store', 'Source': ('ra', 's0-s9'), 'Immediate': '340'}
{'PC': '8fe2a5d8', 'Encoding': 'd2a6', 'WoE': 32, 'Instruction': 'push', 'Category': 'store', 'Source': ('ra', 's0-s1'), 'Immediate': '100'}

Notice that these instructions are all "push" instructions and they are all 32 bit, however the Encoding field seems to be 16 bit in 4 of the cases above. Does the encoding field from the hca script follow this specification? https://github.com/riscv/riscv-code-size-reduction/blob/master/ISA%20proposals/Huawei/Zce_spec.adoc#pushpoppopret

@tariqkurd-repo
Copy link
Contributor

Hi @kjetilos the encoding field is invalid in the case that the instruction has been added by the script and should be clearly marked as such.

@abukharmeh please fix this.

Tariq

@abukharmeh
Copy link
Contributor

abukharmeh commented Jul 12, 2021

Hi Kjetilos,

No the script does not attempt to replace the encoding field with the correct value, it always sets the correct WoE and Instruction for size estimation. Some times it sets additional fields like source and destinations but it never sets the correct encoding.

I think it make sense to mark modified instructions fields with XXXX where they are not replaced to indicate that some parts of the instruction were replaced by an optimization scan, but this is not done at the moment.

Also please note that when you wrote your actions for the script call last git issue, I noticed that you call pushpop twice, once with pushpop then with pushpop,both. You only need to call it once with pushpop,both, the script should then decide what is the best use of the encoding and modify instructions accordingly. I have a feeling that you call pushpop before pushpop,both thus the script replaces all calls with 32bits calls.

Kind regards,
Ibrahim.

@kjetilos
Copy link
Author

Hi Ibrahim,

This explains the Encoding behavior. I will ignore the content of this field for the push and pop instructions.

It sounds like the order of input arguments is important to analyze code size savings correctly with the script. What is the full commandline we should use to measure the total codesize benefits of the Zce extension? Should we only use "pushpop,both" or should we place pushpop after pushpop,both.

@abukharmeh
Copy link
Contributor

abukharmeh commented Jul 14, 2021

Yes the order of the actions is important as they are executed in the same order they are written, and if an action expects instructions that another action changed, then it wont be able to match to them.

You only need to use pushpop,both. I think the way pushpop is splitted now is not the best it can be and I have a patch that slightly improve its behaviour ready to be released, however I am delaying it a bit to make sure I dont have any side effects with #97

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants