Stacktrace not trimmed or trimmed stacktrace ruined by additionnal full stacktrace #1763
Comments
|
Sorry that you're having issues with this! If you want stack trace trimming, you can use E.g., you can do: import chisel3._
import chisel3.stage.ChiselMain
class Foo(param: Boolean) extends Module {
val io = IO(new Bundle {
val in = Input(Bool())
val out = Output(Bool())
})
io.out := ~io.in
throw new Exception("barf")
}
object Main extends App {
class Bar extends Foo(true) {
override def desiredName = "Foo"
}
ChiselMain.main(Array("--module", classOf[Bar].getName) ++ args)
}There was never support for passing parameters directly into your module (thought I've prototyped this)... You should be able to add this in in your own Main object, though. @johnsbrew wrote:
😬 😬 😬 The guarantee is that you'll get the same Verilog for the same set of transforms in the same order. A (Have you experienced issues here?) The split between There's also some organic growth here where the stack trace pruning is jankily swallowing the stack trace and printing a pruned version that pretends it's an actual stack trace, but is actually |
So now you are suggesting a third method that is behaving like neither of both previous ones ... And, looking further into it: Is this really your "solution", catching the culprit I identified earlier?
As far as I digged into this issue, I found no way to avoid file IO since the API change with version 3.4.0 (in particular getting the verilog string without getting the verilog file written) so I don't really get your point here. The only acceptable solution here would be a global (or category-based: anno, fir, v) annotation preventing the actual file IO wherever required.
Well, that's precisely the point, we don't really require the same phases (in Needless to say that I can't accept your answer here: no solutions, just an ugly workaround and no willingness to fix what has definitely been an issue for quite some time: chisel generation API is a wonderful undocumented mess with not only 2 but now 3 different ways to produce apparently the same output but with at least different runtime behaviors... I am sorry about my dramatic tone here, and while I do understand some of the underlying reasons in your answer I haven't seen any improvement regarding this basic but crucial API, and I really want to emphasize how painful it is and insecure it feels working with it. |
|
Hi Jean, I understand that you are frustrated with the current API and its associated inconsistencies, and I too would be frustrated and upset in your case. I think we can all agree that we are all in the business of improving Chisel, and I did want to point out that Schuyler has made many great contributions to the Chisel ecosystem and I am not sure that we should say that he maliciously disregards users and usability - after all, we have all made mistakes in API design and there is certainly always more to learn. Perhaps we could all work together to find a way to better understand the current state of the art and figure out the right path to move forward on. Just 2 cents. |
|
Hi @edwardcwang , I cannot agree more with you :)
So what should be the next steps to work on here?
Is this something I could help with as a first step? Does the idea of disabling file IO with some annotations sounds reasonable? I think I could submit a PR for that. Would this be enough to drop the duplicated implementation of I have unfortunately more ideas than time and API changes might (by design) be quite controversial so let me know if there is something useful I can work on. And once again thank you all for the good work on chisel, I am sorry if anyone felt insulted, it was definitely not my intention at all. |
Annoyingly, this was done to not have to touch the original stack trace trimming implementation (#931). Stack trace trimming works in the following way (summarizing #931):
This defines the stack trace trimming API as, "If you see a magical exception, exit as fast as possible and, by the way, no need to show the user any info because I already printed whatever information they need." I'd like to rework this to something cleaner: actually show a trimmed stack trace.
Clarifying: /* This will cause File IO to happen because it's a Stage */
(new ChiselStage).emitVerilog(new Foo)
/* This will not cause any File IO, it just returns a string. This just a bag of phases. */
ChiselStage.emitVerilog(new Foo)The
I think what the real problem here is that there's no good documentation on how to build standalone tools using the Stage/Phase stuff. I'm willing to help either refactor things or to get some documentation here.
All good man. I'm here to help and want to make things better! (I'm a bit distracted today with other things, but I'm trying to follow the discussion here.) |
|
Using #1771, this behavior is unified:
Consider: package example
object SimpleExceptionIssue extends App {
(new ChiselStage).emitVerilog(new HwClass(doThrow = true), args)
}
object SimpleExceptionIssue2 extends App {
ChiselStage.emitVerilog(new HwClass(doThrow = true))
}
class HwClassNoParameters extends HwClass(true)Running all permutations of this using #1771 locally published: You can also not use your own main and then just use |
|
@johnsbrew wrote:
Sorry, missed this. No need as I went ahead and fixed this in #1771/#1773. This should show up in 3.4.3 (the next minor release). Does this fix your issue? @johnsbrew wrote:
There is now. The policy is roughly that exceptions should not be swallowed or wrapped. They should be trimmed to user code close to the point of origin if possible. The What do you think? @johnsbrew wrote:
Possibly. I think that can be done with an annotation that controls Overall, the rationale is roughly the following:
The split between This overall factoring results in the underlying phases being reusable, but the stages being closer to command line apps that may not compose well together (or will do file IO). Anyway, I think this all warrants discussion. My general thinking is that @johnsbrew wrote:
Suggestions and feedback are always welcome. If you could take the time to write up what you're thinking that would help seed a discussion. @johnsbrew wrote:
Thanks for stating this. |
|
Hi @seldridge
Yes it does, thank you very much for your reactivity on this, you pretty much nailed it with #1771 !
This is great, I really like your
Sorry I get confused here as I am still using some now outdated phase in my flow, I should fix that. (But as you said, the phase system is intended to be integrated in a main where it works well)
I get your point, but as you said earlier
Although I'm willing to help, I don't think I qualify for that one...
Well as you fixed both the initial issue and (by the way!) the entire Exception handling system I can only point out some potential improvements:
Thank you again for your time here! |
|
@johnsbrew wrote:
Good thought! The @johnsbrew wrote:
I just need to stop sandbagging and write the documentation... There's some high-level motivation-type stuff that I wrote up over a year ago here: freechipsproject/www.chisel-lang.org#39. I also started to work up a Scastie example that uses stage/phase stuff out of transforms that operate on @johnsbrew wrote:
This should live for a while as it's a public API. However, the first step would be to deprecate @johnsbrew wrote:
This sounds great. Just adding the annotations parameter would be great. 👍 |
Type of issue: bug report
Impact: no functional change
Development Phase: request
Other information
Although chisel does a great job at trimming stack trace, when throwing an exception during chisel elaboration,
firrtl.options.StageErrorstill does (additionally) throw the whole stack trace.While I do understand it remains very convenient for internal debug purpose, it is quite a pain because the beautiful trimmed stack-trace is hidden from user.
If the current behavior is a bug, please provide the steps to reproduce the problem:
Note that the additional function & custom Exception are not required to demonstrate the behaviour.
There are here only to add some user-space call in the stacktrace.
What is the current behavior?
Note that this becomes even worse when using
object ChiselStagerather thanclass ChiselStagesuch asIn such a case you don't even get the trimmed stacktrace and the user error message is hidden as a cause:
What is the expected behavior?
Please tell us about your environment:
- version:
3.4.1- OS:
Darwin 17.7.0 Darwin Kernel Version 17.7.0: Fri Oct 30 13:34:27 PDT 2020; root:xnu-4570.71.82.8~1/RELEASE_X86_64 x86_64What is the use case for changing the behavior?
Nicer errors = happier users
Issue summary
object&class ChiselStageobject ChiselStagerelies onPhaseManagerclass ChiselStagerelies onfirrtl.options.Stage(who is actually the culprit for throwing the additional stacktrace)For me
ChiselStage(and the entire Stage/Phase API) is one of the most obscure piece of code of the chisel/firrtl ecosystem although, several times, I spend hours (if not days) digging into it just to be able to customize our build flow a little bit...And here we have... a companion object not even using its associated class but both somehow relying on the same underlying Stage/Phase API, with roughly the same result... but not quite!
There is a really serious underlying concern about trust here:
When I first posted about this issue in chipsalliance/firrtl#974 I thought I was looking for a quick fix... but as soon as
Stage/Phase APIgets involved, suprises occur and everything gets cumbersome...So I would be happy to help about the stacktrace issue, but this entire file https://github.com/chipsalliance/chisel3/blob/master/src/main/scala/chisel3/stage/ChiselStage.scala needs some uniformization first... and on that one I would like to request help from @seldridge or @jackkoenig ?
The text was updated successfully, but these errors were encountered: