[JSON] properly emit code for alternative function clause execute syntax

[ThinkOpenly]

Most function clause execute stanzas use a syntax like:

function clause execute (ADDIW(imm, rs1, rd)) = {
  let result : xlenbits = sign_extend(imm) + X(rs1);
  X(rd) = sign_extend(result[31..0]);
  RETIRE_SUCCESS
}

Some use a different syntax:

function clause execute (C_ADDIW(imm, rsd)) =
  execute(ADDIW(sign_extend(imm), rsd, rsd))

This is an indirect reference to the prior function clause execute with some parameter manipulation.

It would be nice to substitute the actual code in place of the reference.

[harshagr70]

hey @ThinkOpenly i would like to work on this issue , please assign it to me !!

[ThinkOpenly]

@harshagr70 , you are welcome to investigate this, but this might not be an easy one. Ask lots of questions.

Interestingly, looking again at the example above, I think there is some unnecessary redundancy. The sign_extend function is called twice for the imm parameter, once before calling ADDIW and once within ADDIW. That's arguably a bug in the Sail source.

[harshagr70]

@ThinkOpenly , i am unable to find the function use , or the function discussed above in the /sail repository , please help me figure it out ! , correctly said there's probably a bug in the function's code as mentioned .

[ThinkOpenly]

i am unable to find the function use , or the function discussed above in the /sail repository

This project has a few related repositories, and I am probably careless in distinguishing among them:

• https://github.com/ThinkOpenly/sail: a fork of the upstream Sail parser that includes the JSON backend.
• https://github.com/ThinkOpenly/sail-riscv: a fork of the upstream RISC-V Sail specification that includes some enhancements to include missing information or make information more easily extracted.
• https://github.com/ThinkOpenly/RISC-V_ISA: the source for a website which displays the extracted information as an interactive assembly language reference.

The functions described in the opening comment for this issue can be found in the second repository:

• https://github.com/ThinkOpenly/sail-riscv/blob/a1c94fe96fd3cfa723df1bd1c62af6f3529a966d/model/riscv_insts_base.sail#L519
• https://github.com/ThinkOpenly/sail-riscv/blob/a1c94fe96fd3cfa723df1bd1c62af6f3529a966d/model/riscv_insts_cext.sail#L161

[harshagr70]

@ThinkOpenly
I’ve submitted a pull request that addresses this issue by implementing the direct C_ADDIW clause and removing redundant operations. Please review it at your convenience.

[ThinkOpenly]

I’ve submitted a pull request that addresses this issue by implementing the direct C_ADDIW clause and removing redundant operations.

I commented in that PR, but copying some of that reply here for clarification as to the expectation for solving this issue...

The preferred solution would be to emit the (duplicated) code in the JSON produced by the Sail JSON backend, without changing the Sail code at all.

[Or,] it could be that the code emitted in JSON currently is just fine, and the burden of linking the indirect call to the actual code should be left to downstream users of the JSON, like that website referenced in #32 (comment)

[ThinkOpenly]

Interestingly, looking again at the example above, I think there is some unnecessary redundancy. The sign_extend function is called twice for the imm parameter, once before calling ADDIW and once within ADDIW. That's arguably a bug in the Sail source.

This has nothing to do with this issue, but since I brought it up here, I thought I should resolve it here.

I investigated this, and it seems this is a necessary evil:

• C_ADDIW is given an immediate value of type bits(6)
• ADDIW expects an immediate value of type bits(12)
• The sign_extend() is used to sign-extend and promote the bits(6) value to a bits(12) value suitable for ADDIW

[harshagr70]

@ThinkOpenly got it !! , i will raise a pr once i fix the issue in the backend json , thanks for follow up @ThinkOpenly !!