riscv / sail-riscv

Sail RISC-V model
https://lists.riscv.org/g/tech-golden-model
Other
429 stars 158 forks source link

Recent Changes in Sail Log has broken riscv isac flow #526

Open UmerShahidengr opened 3 weeks ago

UmerShahidengr commented 3 weeks ago

Dear All, Some recent change in Sail log has completely broken the riscv-isac flow to find the coverage of arch-tests. Previously Sail log used to look like below, and there were 2 new lines between every instruction log. This new line character was getting used by riscv-isac in its algorithm to find the starting point and ending point of each instruction, and to compare it against the covergroups.

# sail old log
mem[X,0x80000128] -> 0x4033
mem[X,0x8000012A] -> 0x03A0
[79] [M]: 0x80000128 (0x03A04033) div zero, zero, s10

mem[X,0x8000012C] -> 0x2023
mem[X,0x8000012E] -> 0x0001
[80] [M]: 0x8000012C (0x00012023) sw zero, 0(sp)
mem[0x80006114] <- 0x00000000

mem[X,0x80000130] -> 0x0893
mem[X,0x80000132] -> 0x1000
[81] [M]: 0x80000130 (0x10000893) addi a7, zero, 256
x17 <- 0x00000100

Now this log has been replaced by the one given below which has no new lines between its instruction log. This has completely messed up riscv-isac, and it is unable to parse the log to identify starting points and ending points of any instruction. It is assuming whole log file as single instruction and cannot evaluate coverage.

# New Sail log with no new lines in between
mem[X,0x80000188] -> 0x4033
mem[X,0x8000018A] -> 0x03A0
[103] [M]: 0x80000188 (0x03A04033) div zero, zero, s10
mem[X,0x8000018C] -> 0x2023
mem[X,0x8000018E] -> 0x0001
[104] [M]: 0x8000018C (0x00012023) sw zero, 0x0(sp)
mem[0x80006114] <- 0x00000000
mem[X,0x80000190] -> 0x0893
mem[X,0x80000192] -> 0x1000
[105] [M]: 0x80000190 (0x10000893) addi a7, zero, 0x100
x17 <- 0x00000100

I request you to please switch back to old log template, and/or notify all stakeholders before doing such changes in Golden Model which could affect the other tools (specially arch-tests). I was assuming that I have broken riscv-isac tool, and it took alot of time to identify this issue.

UmerShahidengr commented 3 weeks ago

@billmcspadden-riscv, your immediate attention is required here.

Alasdair commented 3 weeks ago

As far as I can tell, the commit to remove extra blank lines in the output was made almost exactly one year ago on August 28th 2023, so this is not a recent change:

https://github.com/riscv/sail-riscv/commit/ec6a6a9601b2efe3890c1f23c7753ad9129ec868

I request you to please switch back to old log template, and/or notify all stakeholders before doing such changes in Golden Model

I think this means we need a more serious discussion about generating an actual machine-readable trace format if consuming the traces we generate is very important to downstream projects. We don't necessarily know who our stakeholders are, and it's not reasonable to expect the model trace format to be frozen indefinitely to support ad-hoc parsing decisions by downstream projects.

billmcspadden-riscv commented 3 weeks ago

I'll add this as a topic for our tgmm agenda.

@Alasdair Armstrong @.> , I see that you've already submitted a PR to fix the problem by adding a command line switch to force a newline (a single newline) after each simulator step. @Umer Shahid @.> , does this fix your problem? Will a single newline suffice? And does the command line switch work for you?

Bill Mc.

On Tue, Aug 27, 2024 at 5:49 AM Alasdair Armstrong @.***> wrote:

As far as I can tell, the commit to remove extra blank lines in the output was made almost exactly one year ago on August 28th 2023, so this is not a recent change:

ec6a6a9 https://github.com/riscv/sail-riscv/commit/ec6a6a9601b2efe3890c1f23c7753ad9129ec868

I request you to please switch back to old log template, and/or notify all stakeholders before doing such changes in Golden Model

I think this means we need a more serious discussion about generating an actual machine-readable trace format if consuming the traces we generate is very important to downstream projects. We don't necessarily know who our stakeholders are, and it's not reasonable to expect the model trace format to be frozen indefinitely to support ad-hoc parsing decisions by downstream projects.

— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/526#issuecomment-2312187436, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXROLOA6ZWO2GN7IZXVYJ7DZTRKTNAVCNFSM6AAAAABNFWVYPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJSGE4DONBTGY . You are receiving this because you were mentioned.Message ID: @.***>

-- Bill McSpadden Formal Verification Engineer RISC-V International mobile: 503-807-9309

Join me at RISC-V Summit North America http://www.riscvsummit.com/ in October!

billmcspadden-riscv commented 3 weeks ago

Thanks alot for the quick response Bill and Alasdair. I haven't checked the new switch yet, I will use it and will let you know. It seems like I was using quite an older version of Sail locally while running and debugging ACTs. Today I updated my tools locally (including Sail) to update the CI and to merge all ACT tools together, then I came to know this issue. I was assuming that there was some issue in the ACT tools (particularly riscv-isac) but the issue was somewhere else.

On Tue, Aug 27, 2024, 6:15 PM Bill McSpadden @.***> wrote:

I'll add this as a topic for our tgmm agenda.

@Alasdair Armstrong @.> , I see that you've already submitted a PR to fix the problem by adding a command line switch to force a newline (a single newline) after each simulator step. @Umer Shahid @.> , does this fix your problem? Will a single newline suffice? And does the command line switch work for you?

Bill Mc.

On Tue, Aug 27, 2024 at 5:49 AM Alasdair Armstrong < @.***> wrote:

As far as I can tell, the commit to remove extra blank lines in the output was made almost exactly one year ago on August 28th 2023, so this is not a recent change:

ec6a6a9 https://github.com/riscv/sail-riscv/commit/ec6a6a9601b2efe3890c1f23c7753ad9129ec868

I request you to please switch back to old log template, and/or notify all stakeholders before doing such changes in Golden Model

I think this means we need a more serious discussion about generating an actual machine-readable trace format if consuming the traces we generate is very important to downstream projects. We don't necessarily know who our stakeholders are, and it's not reasonable to expect the model trace format to be frozen indefinitely to support ad-hoc parsing decisions by downstream projects.

— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/526#issuecomment-2312187436, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXROLOA6ZWO2GN7IZXVYJ7DZTRKTNAVCNFSM6AAAAABNFWVYPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJSGE4DONBTGY . You are receiving this because you were mentioned.Message ID: @.***>

-- Bill McSpadden Formal Verification Engineer RISC-V International mobile: 503-807-9309

Join me at RISC-V Summit North America http://www.riscvsummit.com/ in October!