Index to Register File for Compressed Instructions not correctly handled

[fpedd]

If the index to the register file for data writeback is not static (i.e. not constant or not part of the instruction encoding), M2-ISA-R has issues getting the scope right. I guess this is a rare edge case, as it is generally not allowed to index the register file using "runtime data" (normally the source and destination register indexes are part of the instruction itself, correct me if I am wrong here). However, the CIW, CL, CS, and CB encodings of the compressed instruction set allow only a subset of the 32 register file to be accessed. Therefore, they only allow access to 8 registers (hence the 3 bit wide rs1 field below). This is elegantly implemented in the CoreDSL by adding 8 to the 3bit rs1 index, thus allowing access to registers x8-x15.

For example, let us have a look at the compressed shift right logical immediate instruction:

C.SRLI {//(RV32 nse)
    encoding:b100 | b0 | b00 | rs1[2:0] | shamt[4:0] | b01;
    args_disass: "{name(8+rs1)}, {shamt}";
    val rs1_idx[5] <= rs1+8;
    X[rs1_idx] <= shrl(X[rs1_idx], shamt);
}

which generates the following behaviour code:

...
1:etiss_uint32 shamt = 0;
2: static BitArrayRange R_shamt_0(6, 2);
3: shamt += R_shamt_0.read(ba) << 0;
4: etiss_uint32 rs1 = 0;
5: static BitArrayRange R_rs1_0(9, 7);
6: rs1 += R_rs1_0.read(ba) << 0;
7: 
8: // -----------------------------------------------------------------------------
9: 
10:     CodePart & partInit = cs.append(CodePart::INITIALREQUIRED);
11:
12:     partInit.code() = std::string("//C.SRLI\n");
13:
14: // -----------------------------------------------------------------------------
15: partInit.code() += "etiss_uint8 rs1_idx = (" + std::to_string(rs1 + 8) + ") & 0x1f;\n";
16: partInit.code() += "*((RISCV*)cpu)->X[rs1_idx] = (*((RISCV*)cpu)->X[rs1_idx]) >> (" + std::to_string(shamt) + ");\n";
17: partInit.code() += "cpu->instructionPointer = " + std::to_string(ic.current_address_ + 2) + ";\n";
18: // -----------------------------------------------------------------------------
19:
20:     partInit.getRegisterDependencies().add(reg_name[rs1_idx], 32);
21:     partInit.getAffectedRegisters().add(reg_name[rs1_idx], 32);
22:     partInit.getAffectedRegisters().add("instructionPointer", 32);
...

While the "static" variables in lines 1-6 work just fine, the addition in the behaviour part of the instruction in line 15 causes the rs1_idx variable to be in the scope of the jit code. This raises an error in lines 20-21 when rs1_idx is referenced outside the jit code.

Now a simple fix would be to get rid of the rs1_idx variable, like so:

C.SRLI {//(RV32 nse)
    encoding:b100 | b0 | b00 | rs1[2:0] | shamt[4:0] | b01;
    args_disass: "{name(8+rs1)}, {shamt}";
    X[ rs1+8] <= shrl(X[ rs1+8], shamt);
}

However, this would make M2-ISA-R not compatible with the RISC-V CoreDSLs provided by Minres.

Another approach would be to detect if an assignment is completely "static" and then put its "target" variable outside the jit code (this would help ETISS runtime performance as well).

Or one could inline such simple expressions at compile-time, producing my "hand-optimized" code from above,. However, this is probably the most labour intensive approach concerning the required changes to M2-ISA-R.

[wysiwyng]

As I see it, this problem is caused by the recent commits removing static scalars (which of course was necessary due to other issues). A quick fix would be to add an attribute to scalars, allowing manual specification whether a scalar should be treated as non-static. In the long run, the staticness detection of scalars has to be implemented correctly of course.

Changing the CoreDSL files is not that big of a problem, as long as the functionality stays the same.

I tried some stuff with the suggested double (static and non-static) assignments already and came to no satisfying result.

Another more fundamental problem I see here is that ETISS simply does not support the case of register accesses where the index is only known at runtime. Perhaps this should also be an issue over on that repository, @rafzi do you have some comments regarding this?

[fpedd]

In the long run, the staticness detection of scalars has to be implemented correctly of course.

Do you have any concrete idea or concept on how to do that? Or can you recommend any good resources I can also have a look at to get an idea of what you have in mind? Thanks in advance :)

Changing the CoreDSL files is not that big of a problem, as long as the functionality stays the same.

I would see that as the most sensible solution for now.

I tried some stuff with the suggested double (static and non-static) assignments already and came to no satisfying result.

Let me know if I can help you in any way by coming up with solutions or testing stuff.

I just want to point out that the CoreDSL referenced in #3 requires M2-ISA-R to be able to handle such statements. So we need a different solution in the long run. Particularly since the backend won't change significantly for M2-ISA-R CoreDSL 2.0.

Another more fundamental problem I see here is that ETISS simply does not support the case of register accesses where the index is only known at runtime. Perhaps this should also be an issue over on that repository, @rafzi do you have some comments regarding this?

But is that even required? Are there any instructions for which the register index is not known at compile-time?

[wysiwyng]

Do you have any concrete idea or concept on how to do that?

At the moment not really, but probably something along the lines of full static code analysis will be required. The current static detection logic is rather dumb, it does not take scoping into account properly.

I will probably try the double assignments again, this sounds (at least for now) the easiest to implement. I will again have to check whether this works with scoping though.

But is that even required?

Not yet, but it is possible in theory. Another example here: ARM has load / store multiple instructions, which can currently only be implemented as multiple repeated statements and not as loop.

[rafzi]

Another more fundamental problem I see here is that ETISS simply does not support the case of register accesses where the index is only known at runtime. Perhaps this should also be an issue over on that repository, @rafzi do you have some comments regarding this?

This probably would never make sense in an ISA. It'd be horrible for pipelining and many other microarchitectural optimizations.

So it should be safe to assume that the expressions indexing registers are always statically resolvable. However, this might of course get quite complex in some cases like the ARM instructions that reference multiple registers.