On 3/24/2023 12:07 AM, Richard Henderson wrote: > On 3/22/23 23:00, Wu, Fei wrote: >>>> + ctx->priv = env->priv; >>> >>> This is not right. You should put env->priv into tb flags before you use >>> it in translation. >>> >> I see some other env usages in this function, when will env->priv and >> tb_flags.priv mismatch (assume we have recorded priv in tb_flags)? > > You are correct that they should match, because of tb_flags, but it is > bad form to read from env, as that leads to errors. Since you *can* > read the same data from tb_flags, you should. > I lack some background here, why should tb_flags be preferred if env has the same info? Then for reading from tb_flags, we need to add it to tb_flags first.
Correct me if I'm wrong. The only strong reason we add some flag to tb_flags is that it causes codegen generate different code because of this flag change, and it looks like priv is not one of them, neither is mmu_idx, the generated code does use mmu_idx, but no different code generated for it. I think here we have some other reasons to include more, e.g. reference env can be error-prone in some way. So there are minimal flags must be in tb_flags, but we think it's better to add more? Thanks, Fei. > The read of misa_ext and misa_mxl_max are correct, because they are > constant set at cpu init/realize. > > The read of vstart is incorrect. The TB_FLAGS field is VL_EQ_VLMAX, > which includes a test for vstart == 0, but the smaller vstart == 0 test > is not extractable from that. Thus the usage in e.g. > vext_check_reduction is incorrect. One would require a new TB_FLAGS bit > to encode vstart ==/!= 0 alone. > > > r~
