bespoke-silicon-group / bsg_manycore

Tile based architecture designed for computing efficiency, scalability and generality
Other
227 stars 58 forks source link

Icache instr 30b #617

Closed tommydcjung closed 2 years ago

taylor-bsg commented 2 years ago

Instead of threading the parameter through, can we just hide the "11" generation inside the /hard implementation of the SRAM? have we confirmed that we don't have any instructions that use those bits?

dpetrisko commented 2 years ago

Instead of threading the parameter through, can we just hide the "11" generation inside the /hard implementation of the SRAM? have we confirmed that we don't have any instructions that use those bits?

I think it's good to have a parameter, since there should be a check -- once we use those bits for some instruction -- that you don't use that class of instruction while the parameter is set. Otherwise, you'll just get in a situation where RTL passes, but synth fails and that's never a good position to be in...

taylor-bsg commented 2 years ago

In the /hard implementation of the RAM, we can verify that the low two bits are always written? That seems better than what we have now, where we actually don't check anything.

dpetrisko commented 2 years ago

I think we’d be losing some valuable abstraction, since the hardened rams usually don’t have any “application logic” in them and can just be straight replaced by whatever flow the user likes. This would require somewhat esoteric knowledge of RISC-V and the system to take advantage of, whereas encoding it in the source RTL is more clear about the tradeoff.

taylor-bsg commented 2 years ago

I think longer term, the solution is to pass a configuration enum in, just like with BP, to avoid endless proliferation of parameters. But time is short; I think we can do the 11 check hack for now.

M

On Mon, Jan 17, 2022 at 9:09 PM Dan Petrisko @.***> wrote:

I think we’d be losing some valuable abstraction, since the hardened rams usually don’t have any “application logic” in them and can just be straight replaced by whatever flow the user likes. This would require somewhat esoteric knowledge of RISC-V and the system to take advantage of, whereas encoding it in the source RTL is more clear about the tradeoff.

— Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/617#issuecomment-1015075911, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AEN6IYJYWQOQPWE7X3UWTY2HANCNFSM5MBJG6PQ . You are receiving this because your review was requested.Message ID: @.***>

dpetrisko commented 2 years ago

Okay, fine with me. So this PR should keep the sign-bit flops, but revert the changes to the instruction format / RAM?

taylor-bsg commented 2 years ago

That is what makes sense to me..

On Tue, Jan 18, 2022 at 9:15 AM Dan Petrisko @.***> wrote:

Okay, fine with me. So this PR should keep the sign-bit flops, but revert the changes to the instruction format / RAM?

— Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/617#issuecomment-1015634018, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AEFQVLHXFLG2RTH2O3UWWN33ANCNFSM5MBJG6PQ . You are receiving this because your review was requested.Message ID: @.***>

tommydcjung commented 2 years ago

That works for me too

dpetrisko commented 2 years ago

new version LGTM

tommydcjung commented 2 years ago

I checked that the software stack does not write any opcodes not ending in 2'b11, but there could be cases where the host is zeroing out the icache with 32'b0, which is considered as nop.

taylor-bsg commented 2 years ago

@tommydcjung is 30'b0, 11 also a nop?

tommydcjung commented 2 years ago

No, 0000011 is LOAD opcode.

dpetrisko commented 2 years ago

So looks like 000000 is a nop because it doesn't match any opcodes in the system i.e. illegal instruction. We could make the initialization be 1111111 instead, which should similarly decode to nop?

taylor-bsg commented 2 years ago

Is 0^30 11 effectively noop on manycore? I assume it maps to LW $0,0($0), which hits in DMEM?

But actually, I thought 0 was illegal instruction, not noop, in RISC-V?

M

On Tue, Jan 18, 2022 at 12:01 PM Dan Petrisko @.***> wrote:

So looks like 000000 is a nop because it doesn't match any opcodes in the system i.e. illegal instruction. We could make the initialization be 1111111 instead, which should similarly decode to nop?

— Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/617#issuecomment-1015788489, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AGE45JDTKBIVUHQZZ3UWXBK5ANCNFSM5MBJG6PQ . You are receiving this because your review was requested.Message ID: @.***>

dpetrisko commented 2 years ago

'0 is an illegal instruction, but manycore does not trap on illegal. Instead, the decoder simply doesn't match any instructions, so the decode falls through to '0 control signals

dpetrisko commented 2 years ago

(Canonical nop is addi, x0, 0)

taylor-bsg commented 2 years ago

If Tommy can confirm that 0^30 11 behaves as I said then we are good…

On Wed, Jan 19, 2022 at 12:21 PM Dan Petrisko @.***> wrote:

(Canonical nop is addi, x0, 0)

— Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/617#issuecomment-1016837051, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AGP5OTMYBJSNNEGMXTUW4MMLANCNFSM5MBJG6PQ . You are receiving this because your review was requested.Message ID: @.***>

tommydcjung commented 2 years ago

Yes it will just hit DMEM and write back to x0. But the program shouldn't be executing those zero instructions added to align the code any way, so we are good.

taylor-bsg commented 2 years ago

Okay sounds good.

On Wed, Jan 19, 2022 at 2:06 PM Tommy Jung @.***> wrote:

Yes it will just hit DMEM and write back to x0. But the program shouldn't be executing those zero instructions added to align the code any way, so we are good.

— Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/bsg_manycore/pull/617#issuecomment-1016916251, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AENOPMCNDRAYKF7PE3UW4YXHANCNFSM5MBJG6PQ . You are receiving this because your review was requested.Message ID: @.***>