C nits

[riscv]

C nits

[DSHorner]

zimm is used in the description of ADDI4SPN. I wanted to use uimm consistently in the table for unsigned zero extended values, and revise ADDI4SPN accordingly. But zimm appears to be a directive to the assembler: "and expands to addi rd 0 , x2, zimm[9:2]". (as does nzimm by it's usage in "expands to" descriptions, but with sign extension instead). I've been trying to confirm zimm/nzimm directives in binutils, but it could also be a standard macro in any of the projects. Is it safe to switch to uimm as the variant for this document?

[aswaterman]

LI and ADD got fixed last week. I've just fixed SLLI.

Standardizing on simm/nzsimm for sign-extended immediates and imm/nzimm for zero-extended sounds ok; I don't have time to do that at the moment, but would review a PR.

[DSHorner]

I hope to have PR ready tomorrow.

[aswaterman]

Thanks @DSHorner

[sorear]

@DSHorner I'm pretty sure that's just informal notation, not a directive, and we should be consistent with it. There are two axes for an immediate: signed/unsigned, and may-be-zero/must-be-nonzero; everything outside C is a "szimm" but C varies.

[michaeljclark]

Zero extension of RVC instruction immediate values caught me out while working on an RVC implementation based on the spec.

While zero extension is mentioned in the text description for each RVC encoding with a zero-extended immediate, the zero extension property does not appear in tabular form such as in the immediate mnemonic in each instruction's encoding table or in the instruction set listing. The tables just contain 'imm' and don't indicate whether to sign or zero extend. I made frequent reference to both while implementing but the zero extension property can only be derived from the text description of the instruction, and not from the tables.

As @DSHorner mentions, almost all of the Base ISA immediate values are sign extended (excepting shifts and the CSR instruction immediate values), so it is an easy mistake to make. Load and store offsets, including stack relative loads and stores, in the compressed ISA have zero-extended immediate values which is different from the default of sign extension in the Base ISA.

As mentioned: 'z' in zimm (zero-extend?) and 'z' in nzimm (non-zero) are ambiguous as non-zero collides with zero-extend or sign-extend which are on different dimensions so it would be nice to avoid this ambiguity altogether. I think "may be zero" should be implicit. I believed the 'z' in zimm was zero-extend as it was used for the CSR instructions which have zero-extended immediate values (this caught me out too). In any case, it would be nice to have unambiguous mnemonics that appear in a glossary and only use z in nz for non-zero to avoid confusion.

I have been using 'simm' and 'uimm' for signed and unsigned immediate values in metadata derived from riscv-opcodes and note that 'simm' and 'uimm' notation appear in LLVM and CompCert metadata.

This gives an idea of what the instruction listing might look like if we used simm, nzsimm, uimm, and nzuimm for "signed", "non-zero signed", "unsigned" and "non-zero unsigned" respectively:

• https://github.com/michaeljclark/riscv-meta/blob/master/doc/pdf/riscv-instructions.pdf

Note: I don't have the RVC hints as the ISA manual listing does, but it gives you an idea...

[michaeljclark]

Didn't mean to write such a long comment, but it took me a while to track down a sign vs zero extend bug; realize @sorear kind of pointed this out succinctly in the first line of the first comment.

[aswaterman]

I think this is mostly resolved, but feel free to submit another issue (or preferably a PR) if not.