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

Exn #71

Open
wants to merge 44 commits into
base: main
Choose a base branch
from
Open

Exn #71

wants to merge 44 commits into from

Conversation

dz333
Copy link
Collaborator

@dz333 dz333 commented Jun 23, 2022

This is tracking the Exception feature as documented by the docs/exceptions.md file.

Copy link
Collaborator Author

@dz333 dz333 left a comment

Choose a reason for hiding this comment

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

@yy665 See my comments for required changes.


val new_m = addExnVars(m)
new_m.name.typ = m.name.typ
print(new_m.body)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Delete this print statement before merging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

addExnVars should probably just take a Command and return a new Command and only be applied to m.body

This lets us avoid making like 3 new ModuleDefs.

src/main/scala/pipedsl/passes/ExnTranslationPass.scala Outdated Show resolved Hide resolved
src/main/scala/pipedsl/passes/ExnTranslationPass.scala Outdated Show resolved Hide resolved
src/main/scala/pipedsl/passes/ExnTranslationPass.scala Outdated Show resolved Hide resolved
m.copy(body = CSeq(IStageClear(), convertPrimitives(m.body)), commit_blk = m.commit_blk, except_blk = m.except_blk)
}

def convertPrimitives(c: Command): Command = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe change the name or at least add a javadoc style comment - it's not really clear what this is doing from the name. (It's converting exception call arguments and inserting stage kill commands, right?)

src/main/scala/pipedsl/passes/ExnTranslationPass.scala Outdated Show resolved Hide resolved
src/main/scala/pipedsl/typechecker/BaseTypeChecker.scala Outdated Show resolved Hide resolved
src/main/scala/pipedsl/Main.scala Outdated Show resolved Hide resolved
src/main/scala/pipedsl/typechecker/BaseTypeChecker.scala Outdated Show resolved Hide resolved
src/test/scala/pipedsl/ExceptionSuite.scala Outdated Show resolved Hide resolved
src/test/scala/pipedsl/package.scala Outdated Show resolved Hide resolved
src/test/tests/exception/exn-recovery-spec.pdl Outdated Show resolved Hide resolved
verify(s, pc + 1);
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

adding in stage separators (with nothing between them) can be a way to ensure we have more speculative or excepting threads that need to be killed so that the tests are meaningful.

src/test/tests/exception/exn-recovery.pdl Outdated Show resolved Hide resolved
}
}
---
commit:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this commit block is kinda early - normally we would want it to be the final stage. I don't think we're testing anything interesting in this example (other than having more code than the little tests).

We should probably have the commit block be only the final stage (so that there are rf.write and dmem.write calls before the commit block that actually need to be aborted)

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