> > It seems to me that the C extension can be enabled at any point, since > if C is > > off, you know that the next insn is aligned modulo 4. > > >
Ok, This is mostly right. When C extension is enabled 32-bit base instructions can be aligned on 2 bytes boundaries instead of 4 bytes only. So multiple enables and disables of C bit at different code areas theoretically may require this check on C extension enable. I'm not really sure, may be this is might not be a practical use scenario. > It is only if the C extension is enabled, and you want to disable it, > that is > > when we must check to see if the next insn is aligned mod 4. It is > trivial to > > arrange for a particular instruction to be aligned, via assembler > directives. > > So it seems silly to make the definition of the csr write to misa any > more > > complicated than it is. > > I completely agree with you that C extension disable should have alignment check. > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index 960d2b0aa9..8726ef802e 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -370,10 +370,11 @@ static int write_misa(CPURISCVState *env, int > csrno, > > target_ulong val) > > val &= ~RVD; > > } > > > - /* Suppress 'C' if next instruction is not aligned > > - * TODO: this should check next_pc > > + /* > > + * Suppress 'C' if next instruction is not aligned. > > + * We updated env->pc to the next insn in the translator. > > */ > > - if ((val & RVC) && (GETPC() & ~3) != 0) { > > + if ((val & RVC) && (env->pc & ~3) != 0) { > > val &= ~RVC; > > } Just a hint, (env->pc & 3) instead of (env->pc & ~3) , right ? > Thanks, Ahmed