Implement proposed new function pointer fields for obj_t.#527
Implement proposed new function pointer fields for obj_t.#527devinamatthews merged 4 commits intomasterfrom
Conversation
|
I guess I should have made sure it build first :). I'll force-push a working version. |
The added fields: 1. `pack_t schema`: storing the pack schema on the object allows the macrokernel to act accordingly without side-channel information from the rntm_t and cntx_t. The pack schema and "pack_[ab]" fields could be removed from those structs. 2. `void* user_data`: this field can be used to store any sort of additional information provided by the user. The pointer is propagated to submatrix objects and copies, but is otherwise ignored by the framework and the default implementations of the following three fields. User-specified pack, kernel, or ukr functions can do whatever they want with the data, and the user is 100% responsible for allocating, assigning, and freeing this buffer. 3. `obj_pack_fn_t pack`: the function called when a matrix is packed. This functions receives the expected arguments, as well as a mdim_t and mem_t* as memory must be allocated inside this function, and behavior may differ based on which matrix is being backed (i.e. transposition for B). This could also be achieved by passing a desired pack schema, but this would require additional information to travel down the control tree. 4. `obj_ker_fn_t ker`: the function called when we get to the "second loop", or the macro-kernel. Behavior may depend on the pack schemas of the input matrices. The default implementation would perform the inner two loops around the ukr, and then call either the default ukr or a user-supplied one (next field). 5. `obj_ukr_fn_t ukr`: the function called by the default macrokernel. This would replace the various current "virtual" microkernels, and could also be used to supply user-defined behavior. Users could supply both a custom kernel (above) and microkernel, although the user-specified kernel does **not** necessarily have to call the ukr function specified on the obj_t. Note that no macros or functions for accessing these new fields have been defined yet. That is next once these are finalized. Addresses https://github.com/flame/blis/projects/1#card-62357687.
devinamatthews
force-pushed
the
obj_t_makeover
branch
from
5c5193f to
64a1f78
Compare
|
@fgvanzee I've made the changes we discussed. Ready for you to check formatting. |
| struct obj_s; | ||
| struct cntx_s; | ||
| struct rntm_s; | ||
| struct thrinfo_s; |
There was a problem hiding this comment.
I didn't notice these during our Zoom review. Is this part of resolving the circular dependency?
There was a problem hiding this comment.
Are they needed? What would happen if you removed them?
I feel like in past situations like this, I was able to get by without them, though I defer to you as to explaining why that might have been.
There was a problem hiding this comment.
struct obj_s is definitely needed. The others are also needed unless you want to change the order in which the headers are included in blis.h.
There was a problem hiding this comment.
Actually, in C maybe you can omit them? I am used to C++ where there is an actual type system :).
There was a problem hiding this comment.
When you remove them you get:
include/haswell/blis.h:1880:14: warning: declaration of 'struct obj_s' will not be visible outside of this function [-Wvisibility]
struct obj_s* a,
^
which isn't technically an error but doesn't sound good.
There was a problem hiding this comment.
Agreed, we should leave them in. I was just curious.
|
@fgvanzee are you ready for merge after the whitespace tweaks? |
Yarrr. |
Details: - Moved miscellaneous language-related definitions, including defs related to the handling of the 'restrict' keyword, from the top half of bli_macro_defs.h into a new file, bli_lang_defs.h, which is now #included immediately after "bli_system.h" in blis.h. This change is an attempt to fix a report of recent breakage of C++ compilers due to the recent introduction of 'restrict' in bli_type_defs.h (which previously was being included *before* bli_macro_defs.h and its restrict handling therein. Thanks to Ivan Korostelev for reporting this issue in #527. - CREDITS file update.
3 participants
The added fields:
pack_t schema: storing the pack schema on the object allows the macrokernel to act accordingly without side-channel information from the rntm_t and cntx_t. The pack schema and "pack_[ab]" fields could be removed from those structs.void* user_data: this field can be used to store any sort of additional information provided by the user. The pointer is propagated to submatrix objects and copies, but is otherwise ignored by the framework and the default implementations of the following three fields. User-specified pack, kernel, or ukr functions can do whatever they want with the data, and the user is 100% responsible for allocating, assigning, and freeing this buffer.obj_pack_fn_t pack: the function called when a matrix is packed. This functions receives the expected arguments, as well as a mdim_t and mem_t* as memory must be allocated inside this function, and behavior may differ based on which matrix is being backed (i.e. transposition for B). This could also be achieved by passing a desired pack schema, but this would require additional information to travel down the control tree.obj_ker_fn_t ker: the function called when we get to the "second loop", or the macro-kernel. Behavior may depend on the pack schemas of the input matrices. The default implementation would perform the inner two loops around the ukr, and then call either the default ukr or a user-supplied one (next field).obj_ukr_fn_t ukr: the function called by the default macrokernel. This would replace the various current "virtual" microkernels, and could also be used to supply user-defined behavior. Users could supply both a custom kernel (above) and microkernel, although the user-specified kernel does not necessarily have to call the ukr function specified on the obj_t.Note that no macros or functions for accessing these new fields have been defined yet. That is next once these are finalized. Addresses https://github.com/flame/blis/projects/1#card-62357687.