Bug 712 - introduce XLEN-friendly helper class which aggregates all helpers
Summary: introduce XLEN-friendly helper class which aggregates all helpers
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- enhancement
Assignee: Luke Kenneth Casson Leighton
URL:
Depends on:
Blocks:
 
Reported: 2021-09-25 17:11 BST by Dmitry Selyutin
Modified: 2021-12-09 14:01 GMT (History)
2 users (show)

See Also:
NLnet milestone: NLNet.2019.10.Standards
total budget (EUR) for completion of task and all subtasks: 500
budget (EUR) for this task, excluding subtasks' budget: 500
parent task for budget allocation: 241
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:
dmitry = 325 maciej = 75 [lkcl] amount = 100 submitted = 2021-12-09


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Selyutin 2021-09-25 17:11:12 BST
src/openpower/decoder/helpers.py currently has a lot of functions here and there. Putting aside massive global scope pollution, it also introduces another problem: it's completely unusable when some of the functions must be updated so that they take an advantage of XLEN variable. Since XLEN is not part of global scope, it's impossible for instructions to make use of XLEN.
The solution is to distribute everything as part of a class which is XLEN-aware. Once functions are converted into methods, they can access class internals. The created class is supposed to be inherited by ISACaller.

The works include the following steps:

* create a stub class with methods which call the routines from the global scope
* teach decoder/pseudo/parser.py to deal with these methods instead of global routines
* once parser calls all routines from the class, move all the gory implementation details from global scope into the methods
Comment 2 Luke Kenneth Casson Leighton 2021-09-25 18:02:16 BST
the basis of this one is as dmitry says, XLEN needs to be
available to all helpers, and we are not going to
modify the functions to add it as an argument.
so the only other way is to drop all helper routines
(including those compiled by pyfnwriter) into a class,
which ISACaller happens to inherit from.


that in turn means that a list of all helper
routines needs to be created....

OR

the helper.py class actually imported and introspection
used.

for pyfnwriter functions that technique will
not work because the routines themselves are
in fact autogenerated *by* the parser, so will
have to be handled differently.

this all unfortunately needs to be done in "one hit" so sigh
a branch is, reluctantly, probably the most sensible option.
Comment 3 Dmitry Selyutin 2021-09-25 21:15:44 BST
The class with the redirection logic is submitted. Functions will migrate from the global scope on demand to minimize changes at this stage.
Comment 4 Luke Kenneth Casson Leighton 2021-09-25 21:42:23 BST
looks great dmitry, pyfnwriter should be checked, then i think
we're good.
Comment 5 Dmitry Selyutin 2021-09-26 12:08:08 BST
I thought about these for a while, they have one problem. We run pywriter and pyfnwriter as separate stages; one doesn't know of another. We can for sure collect names which we met so far in pyfnwriter stage; but these are unavailable outside of pyfnwriter, so pywriter have no idea which names should be checked w.r.t. "self." prefix addition before the call. Any ideas?

The simpler option would be to add "self." prefix unconditionally and distribute everything as part of our class; I feel, however, that this might bring other problems in the future. What's your take on this?

If the first option is good, I have a patch which already takes care of all `def` sections for pywriter. Perhaps `pyfnwriter` could expose these as part of build or maybe via special option, and `pywriter` could re-use the list.
Comment 6 Luke Kenneth Casson Leighton 2021-09-26 12:27:43 BST
pyfnwriter should be generating "self." in front of
everything, yes. however, annoyingly, this looks like
either a 2 pass situation or just having an explicit
list.

BCD_TO_DPD etc temporarily need to be added to the list