(Hopefully Constructive) Feedback on Silice

[tommythorn]

Congratulation and thanks for Silice. It's an exciting project and I'm impressed with the wealth of examples and supported platforms.

I started playing with Silice and wanted to share my observations and filing an Issue seemed a good way to do this. Instead of filing individual issues (which I'll be happy to do), I wanted to share my experience chronologically, from the point of view (Linux biased).

1. Silice's build instructions didn't work on macOS (wasn't a show-stopper)

2. Trying to generate bitstreams wasn't documented and seems to assume edialize but this isn't documented. Adding a '-t shell' manually to silice-make.py works.

3. There's a lot of Windows-ism everywhere: tabs/space inconsistencies, .md documents with crazy long lines (conventionally we keep lines under 80 chars), trailing white-spaces (nerd-rage). Strongly recommend catching this automatically (mv .git/hooks/pre-commit{.sample,})

4. The divstd_bare example doesn't match the documentation, reverted it back to before it was changed fixed it.

5. It appears impossible to simultaneously use Silice from different architectures as the location of the executable is used to compute the location of libraries. Something more traditional would be preferred. (Yes, I actually use multiple different archs interchangeably {RV64GC,Arm64,x64}x{Linux,[FreeBSD,]macOS}). For now I clone the bin directory per architecture, but it's awkward.

6. Silice bears a lot of resemblance to Timogriffer-C and moderately to Handel-C. The FSM aspect is straight forward, but the fork-join approach is interesting.

   The pipeline concept is particularly interesting. It's not documented but it appears that there is an implicit "valid" bit in pipe (to be checked). I played with this and wrote more about it in https://github.com/tommythorn/silice-examples.

7. silice-make.py doesn't stop building upon source errors

8. Doesn't catch illegal zero-fields, like xxx[4,0], but Verilator complains

9. Comments pipelines doesn't work (compilation fails).

[sylefeb]

Hi - thank you very much for the detailed feedback, much appreciated! These are all good points, I'll be looking into improving each of these aspects (I'll provide details here as I do, and open individual issues if something requires a separate discussion). Thanks also for sharing your example design!

Two small questions:

• for 5., would post-fixing the executable with the platform name be a viable approach?
• for 6., I could not find a lot of info online regarding Timogriffer-C, I'd be interested in learning more about it, is there a paper/webpage describing it?

[tommythorn]

I forgot to say that I'd be happy to fix (almost) all of this and I already have in my sandbox.

5. Hmm. I would probably prefer if the installation process was more conventional and installed into $prefix/bin and $prefix/share or $prefix/lib etc, but I have a workaround for now.

6. Oops, it was Transmogriffer-C. The project was reborn again as FpgaC as is documented here: https://en.wikipedia.org/wiki/FpgaC

As for the other points:

1. building on macOS is only blocked on Java; there are many ways around it, I just don't know the Right One. I'd punt on this until the answer becomes clear.

2. I'm not sure what you wanted to do here so I haven't done anything, but manually run a long make-silice.py command as needed. I'd personally rather limit the already large set of dependencies.

3. I could obviously easily fix this, but I wouldn't want to step on your toes.

4. I can fix this. Appears to be a mistake.

5. Oh, this was written before I understood how it worked and wasn't supposed to have been included. HOWEVER, I'm not very happy with the generated code; the pipeline registers assignments are all of this form

   _q___pip_140_2_pc <= (reset | ~in_run) ? 0 : _d___pip_140_1_pc;

   even when r0_val has been marked as uninitialized. I was expecting the much cheaper

   
   _q___pip_140_2_pc <= _d___pip_140_1_pc;

This should probably be a separate issue.

7 - 9 seems like just simple bugs.

Finally, kindly glance at https://github.com/tommythorn/silice-examples and let me know if it's sub-optimal.  I intend to build on this and would rather fix the issues while the code is simple.

Amicalement

[sylefeb]

Thanks again! I have merged the PR (in draft, I usually work in draft and merge things towards master after more testing). 80 columns no trailing spaces sounds good, I agree, I have to tweak my tools+git to help me with this (ongoing!).

I'll be looking into fixing these bugs, good catch on pipeline + init. Definitely need help with macOS builds.

I'll have a closer look at your design asap (with a fresh mind tomorrow ;) )

Amicalement

[sylefeb]

Hi - here is my rework of your design: https://gist.github.com/sylefeb/901e50040e2e0c09b7ff3a4c43799123

See comment at the end for details, some changes are mostly to show some features but the original was just fine as-is.

Btw, I have a rework of BRAM access planned to make it more 'table like' and not have to always use wenable / addr / rdata / wdata.

[rob-ng15]

The BRAM access changes sound good.

On Mon, 13 Sep 2021, 13:36 sylefeb, @.***> wrote:

Hi - here is my rework of your design: https://gist.github.com/sylefeb/901e50040e2e0c09b7ff3a4c43799123

See comment at the end for details, some changes are mostly to show some features but the original was just fine as-is.

Btw, I have a rework of BRAM access planned to make it more 'table like' and not have to always use wenable / addr / rdata / wdata.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sylefeb/Silice/issues/178#issuecomment-918147954, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN4SYTZAEGN2AJCBY6PXRF3UBXV55ANCNFSM5D3GDQ2Q .