Conversation
tilk
added
enhancement
| m_csr.mcause.write(m, data=1 << (self.gen_params.isa.xlen - 1)) | ||
| with m.Else(): | ||
| # RISC-V synchronous exceptions - don't retire instruction that caused exception, | ||
| # and later resume from it. | ||
| # Value of ExceptionCauseRegister pc field is the instruction address. | ||
| m.d.av_comb += commit.eq(0) | ||
|
|
||
| m.d.av_comb += cause_entry.eq(cause_register.cause) | ||
| cause_entry = Cat( | ||
| cause_register.cause, C(0, self.gen_params.isa.xlen - len(cause_register.cause)) | ||
| ) | ||
| m_csr.mcause.write(m, data=cause_entry) |
There was a problem hiding this comment.
I would not like this particular logic to be changed, because previous way will look nicer with misprediction support (when some causes don't trigger architecture trap - this could be handled under one If with mepc and trap_entry)
Find places where a workaround for single call per body was
Other places that I remember this change would be useful are CSRUnit, and JumpBranchUnit (both related to report method)
lekcyjna123
left a comment
lekcyjna123
left a comment
There was a problem hiding this comment.
Overall it is great change. I left some comments in code and please, add some documentation, docstrings, comments, markdown even in Quenya, but something is very needed.
| def transactions_exclusive(trans1: Transaction, trans2: Transaction): | ||
| tms1 = [trans1] + method_map.methods_by_transaction[trans1] | ||
| tms2 = [trans2] + method_map.methods_by_transaction[trans2] | ||
|
|
There was a problem hiding this comment.
I don't see yet how the exclusiveness of methods propagates to exclusiveness of transactions.
There was a problem hiding this comment.
Lets take such example:
with transaction().body(): #parent
method1()
with m.If(1):
method2()
with m.Else():
with transaction.body(): #child
method1()
with m.If(1):
method2()Are parent and child transaction exclusive or not? According to the below comment they should be exclusive because there exists the method2 that is exclusive. But in parallel we have call to method1 which is obviously wrong and shouldn't be allowed.
There was a problem hiding this comment.
They are exclusive and the child transaction will never run. This situation was not detected by the previous check, and is still not detected in this PR. I think this should be considered a separate issue.
lekcyjna123
left a comment
lekcyjna123
left a comment
There was a problem hiding this comment.
Thanks. Now it is much cleaner.
Co-authored-by: piotro888 <piotro@piotro.eu>
3 participants
This work in progress PR is intended to improve readability and performance of code where Amaranth branches interact with Transactron. In particular:
TODO:
Closes #549.