On 9/3/21 6:30 PM, Peter Maydell wrote:
Let me try to write out the rules:
* if you do something that changes the TB flag, and you know for
definite the new state, and this change happens and is the same
for every exit from the TB, then you can just update the flag
and keep going in this TB. (This is why lspact etc are OK)
* if you don't know for definite the new state, because it might
not have happened, eg a helper function condition changes
something, then you must end the TB immediately (in a way that
doesn't use goto_tb). This is true both for "I couldn't figure
out the new state, it's too hard" and "the new state depends on
some runtime data such that different executions of the same TB
might end up with different values for the flag"
* it's not good enough to say "well, I can pessimistically always
assume mve_no_pred", because of the goto_tb issue. You have to
be sure of the exact value that a TB flags calculation after
the insn will get.
Correct.
So I think that pretty much everywhere in my current patch that is setting
s->mve_no_pred = false needs instead to just end the TB. That seems mostly
straightforward, but there are some tricky cases:
* WLSTP. The code generated for this insn has two exits. The change
to ltpsize happens on only one of those, which is currently
handled by gen_jmp_tb(). I think we are OK to just leave the
code as it is, because the value we pass to LTPSIZE is
constant (encoded in the instruction), and so the new value
of MVE_NO_PRED is always the same on that exit from the TB
for all executions
Correct.
* gen_update_fp_context() -- this function gets called for pretty
much every FP/MVE insn (as part of vfp_access_check), and it
can in rare cases update the FPSCR.LTPSIZE and the VPR. I guess
this means we really do need to end the TB
if (MVE && s->v7m_new_fp_ctxt_needed) (ie the comment in
gen_update_fp_context "We don't need to arrange to end the TB,
because [we don't cache FPSCR in TB flags]" is no longer true).
That seems likely to be painful because some of the insns that
do a vfp_access_check do complicated things with the TB exits
(eg WLSTP, LETP) that are going to override a naive setting of
is_jmp in gen_update_fp_context()...
Well, we could let gen_goto_tb see that is_jmp is already UPDATE_NOCHAIN, and suppress the
goto_tb in that case. That would seem to take care of any entry into gen_jmp(_tb).
r~