On Fri, 3 Sept 2021 at 16:28, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 9/3/21 4:17 PM, Peter Maydell wrote: > > I don't really understand what you mean here. What's the difference > > between ending the TB now and translating a few more insns in > > this TB before we end it? > > VCMP (pred now on or off) > B label > > The code we emit for B uses goto_tb. > > When goto_tb is unlinked (e.g. first execute), we exit with a code that > causes the current > cpu state to be evaluated, the new TB created, and the link filled in. This > makes > permanent the cpu state we evaluated, and thus the state change across > goto_tb must be > constant. This is fine for B, since only the PC changes and to a known > destination. > > So we have two choices: end the TB immediately after the VCMP, or set another > flag to > cause gen_goto_tb to fall back to goto_ptr.
Ah, I see, and when we end after the VCMP then we use UPDATE_NOCHAIN which causes us to use lookup_and_goto_ptr instead of goto_tb. I was wondering if this is also a problem for the flags like s->v7m_lspact, which we currently handle in about this way -- set from a TB flag, but then updated for the rest of the TB. 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. 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 * 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()... * gen_preserve_fp_state() is similar to gen_update_fp_context() thanks -- PMM
