Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: execute() exposes ExecutionReport #847

Merged
merged 14 commits into from
May 30, 2024
Merged

feat: execute() exposes ExecutionReport #847

merged 14 commits into from
May 30, 2024

Conversation

mattstam
Copy link
Contributor

allows callers can check cycles, also removes cycle check / simulation from proof requests

Copy link
Member

@ctian1 ctian1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized wherever we add to the report, we also need to check if unconstrained mode is on

self.total_instruction_count()
)?;
write!(f, "Syscall Counts:\n")?;
for (syscall, count) in &self.syscall_counts {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be nice to sort this in alphabetical order so its consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Contributor Author

@mattstam mattstam May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instruction Counts:
  add: 583439
  and: 151701
  auipc: 19488
  beq: 26917
  bge: 237
  bgeu: 5917
  blt: 1141
  bltu: 43457
  bne: 51442
  divu: 6
  ecall: 2264
  jal: 5372
  jalr: 38749
  lb: 10723
  lbu: 57950
  lw: 237094
  mul: 4036
  mulhu: 1152
  or: 301944
  sb: 58448
  sll: 278698
  sltu: 16766
  srl: 293010
  sub: 12382
  sw: 312781
  xor: 242242
Total Instructions: 2757356
Syscall Counts:
  COMMIT: 8
  COMMIT_DEFERRED_PROOFS: 8
  HALT: 1
  SHA_COMPRESS: 1091
  SHA_EXTEND: 1091
  WRITE: 65
Total Syscall Count: 2264

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the difference in lower vs upper case is annoying, is it fine if I re-write all the Opcode enums

pub const fn mnemonic(&self) -> &str {
to uppercase?

Copy link
Member

@ctian1 ctian1 May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you just do .to_uppercase()? also it looks fine imo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for consistency, should we show 0 for the missing ones?

@mattstam mattstam requested a review from ctian1 May 30, 2024 22:19
@mattstam mattstam merged commit 99effb1 into dev May 30, 2024
5 checks passed
@mattstam mattstam deleted the mattstam/report branch May 30, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants