Add Sail code from Nambi to documentation

[Alasdair]

Hi,

I was requested to demonstrate some new Sail + Asciidoc documentation functionality by creating a pull request on this repository.

I think the Sail code that has been written is not in a state where it can be merged, as most of the Sail code is not actually functional, and some instructions were skipped because the including it in this state would not be useful. Nevertheless, this pull request should be instructive, and we can discuss with Nambi the steps needed to make it mergeable.

[tariqkurd-repo]

Is there a PR for the SAIL code yet?

[tariqkurd-repo]

@Alasdair can you fix the build error? Sorry for the slow reply about this.

[Alasdair]

I just pushed a new version of the gem (0.2) and updated the Makefile

I think there was an issue where wavedrom-cli doesn't work with binary literals and WaveDromEditor does, which could cause build errors based on which was in your environment. I've made it only use hex now, so that shouldn't happen.

[abukharmeh]

@Alasdair

We would also need to add that gem to the dependencies file

[tariqkurd-repo]

undefined method `absolute_path?' for File:Class
Did you mean?  absolute_path
  Use --trace to show backtrace
make: *** [Makefile:9: build] Error 1
Error: Process completed with exit code 2.

any ideas?

[Alasdair]

That's because your CI is using Ruby 2.X rather than Ruby 3.X, is there a reason for that? All Ruby 2.X versions are now EOL and are no longer receiving updates and security fixes as far as I am aware.

[tariqkurd-repo]

That's because your CI is using Ruby 2.X rather than Ruby 3.X, is there a reason for that? All Ruby 2.X versions are now EOL and are no longer receiving updates and security fixes as far as I am aware.

I really have no idea I wasn't involved in the setup, @cetola ?

[Alasdair]

I can probably make things work with the older version if needed, as it's likely just some missing library methods.

[jnk0le]

You need to update ruby to 2.7 at least. asciidoc-pdf requires 2.7 as minimum since about a year and recently started using 2.7+ ruby features.

[abukharmeh]

Bill Traynor just changed the CI to use Ruby version 3.2.0, @Alasdair can we rebase so we hopefully see the output PDF file :)

[tariqkurd-repo]

@Alasdair can you rebase? thanks!

[abukharmeh]

@tariqkurd-repo I synced with my fork and applied the PR and was able to generate the PDF using CI, here is how it will look https://github.com/abukharmeh/riscv-code-size-reduction/releases/tag/Test_SAIL_Gen if you would like to have a look.

Like Alasdair said in the original PR message, the execute clauses do not appear to be completely correct, but maybe we can start by adapting the encoding wavedroms as they are simpler to get right and verify from SAIL source.

I am not sure what would be the acceptance criteria for us to merge the execute clauses and endorse them in the actual spec until we have a valid ACT and the SAIL code is verified against them !

That aside the Sail -> Asciidoc gem appears to be working nicely !

[Alasdair]

Should be rebased now

[abukharmeh]

@tariqkurd-repo Was this merged prematurely, no change log entry, rendering issue for c.zext.h execute clause and most other execute clauses are incomplete/ untested ! If we have to create a new release before all of this is fixed, I would be nervous having this PR included ?

[tariqkurd-repo]

The idea was to merge it to help prove the flow. The SAIL for Zc is imminent, so I hope we can complete this work soon, and check it renders correctly etc. As ratification is due I'd be very dissapointed if we had to make any changes before the SAIL is ready.

[tariqkurd-repo]

see also https://github.com/riscv/sail-riscv/pull/255

[jjscheel]

This PR needs to be reverted due to the SAIL re-work that is on-going and the need to release a Ratified version based on the Frozen copy. Any takers?

[jjscheel]

@rpsene is helping me work around the revert by branching and merging the commits back into the tree ahead of the PR. We will post a final confirmation that no action is needed when we're done.

[jjscheel]

Looks like we're good to go. No action needed here at my request. Document should now build in a "ratified" state.

@tariqkurd-repo, you probably want to set version in your main branch to something other than 1.0 now, perhaps 1.1_preX. You can leave state as "Ratified", however.

[tariqkurd-repo]

Thanks Jeff - delighted to be ratified at last. I'll update the main branch once we merge the SAIL code.

Tariq

On Thu, Apr 27, 2023 at 5:52 PM Jeff Scheel @.***> wrote:

Looks like we're good to go. No action needed here at my request. Document should now build in a "ratified" state.

@tariqkurd-repo https://github.com/tariqkurd-repo, you probably want to set version in your main branch to something other than 1.0 now, perhaps 1.1_preX. You can leave state as "Ratified", however.

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-code-size-reduction/pull/217#issuecomment-1526027776, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOCTJAEIWTTOXS5GXLUPM63XDKP3TANCNFSM6AAAAAAWOQ6QXU . You are receiving this because you were mentioned.Message ID: @.***>

[tariqkurd-repo]

Ideally we'll get the SAIL updated and then it won't be an issue. Nambi?

On Sat, Jun 10, 2023 at 2:38 PM Ibrahim Abu Kharmeh < @.***> wrote:

Can we revert this PR please @tariqkurd-repo https://github.com/tariqkurd-repo, I am troubled that the repo contains really wrong SAIL source code if anyone ever tries to build from source !

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-code-size-reduction/pull/217#issuecomment-1585668683, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOCTJAEDZIWL4OJTLW22OPTXKR2DZANCNFSM6AAAAAAWOQ6QXU . You are receiving this because you were mentioned.Message ID: @.***>