platform: add a mechanism for AFU's to set platform macros #3130
Conversation
Due to the order that SystemVerilog header files are processed, setting a macro in an AFU's sources file may not affect the FIM's afu_main(). Add support for writing macros to the PIM interface header file that afu_main() includes explicity. Signed-off-by: Michael Adler <michael.adler@intel.com>
| if ('define' in afu_ifc_db): | ||
| afu_ifc_db['define'] += afu_ifc_req['define'] | ||
| else: | ||
| afu_ifc_db['define'] = afu_ifc_req['define'] |
There was a problem hiding this comment.
If this is concatenating lists, would this pattern work?
| if ('define' in afu_ifc_db): | |
| afu_ifc_db['define'] += afu_ifc_req['define'] | |
| else: | |
| afu_ifc_db['define'] = afu_ifc_req['define'] | |
| afu_ifc_db.setdefault('define', []) += afu_ifc_req['define'] |
There was a problem hiding this comment.
Yours does look better, but the base object doesn't have setdefault(). I think it's just a list.
There was a problem hiding this comment.
Is afu_ifc_db not a dict? https://docs.python.org/3/library/stdtypes.html#dict.setdefault
There was a problem hiding this comment.
You are of course right, it's a dict. But it looks like setdefault() can't be used as an l-value, so can't be the left side of +=.
There was a problem hiding this comment.
Indeed, thanks. This should work:
| if ('define' in afu_ifc_db): | |
| afu_ifc_db['define'] += afu_ifc_req['define'] | |
| else: | |
| afu_ifc_db['define'] = afu_ifc_req['define'] | |
| afu_ifc_db.setdefault('define', []).extend(afu_ifc_req['define']) |
There was a problem hiding this comment.
I find that harder to understand than the original. Is it important to you?
There was a problem hiding this comment.
Not important; merely a preference to avoid repetition.
There was a problem hiding this comment.
If the first suggestion had actually worked I'd be totally on board. Thanks for trying.
There was a problem hiding this comment.
There is a small semantic difference; the first list afu_ifc_req['define'] is modified as well when a second list afu_ifc_req['define'] is concatenated, since = does not create a deep copy. I assume it does not make a difference here though.
Pull Request Test Coverage Report for Build 9473843507Details
💛 - Coveralls |
Due to the order that SystemVerilog header files are processed, setting a macro in an AFU's sources file may not affect the FIM's afu_main(). Add support for writing macros to the PIM interface header file that afu_main() includes explicity.