miden-vm icon indicating copy to clipboard operation
miden-vm copied to clipboard

always include debug info when assembling

Open plafer opened this issue 11 months ago • 6 comments

Context: https://github.com/0xMiden/miden-base/pull/1353#pullrequestreview-2835612467

Note that without any mitigation this will slow down the fast processor, which executes all decorators. Hence this is related to the issue of only executing the decorators in the fast processor in some "debug mode" (or not at all if that ends up really not being possible).

plafer avatar May 16 '25 14:05 plafer

Thinking about this a bit more, would simply changing the default to "debug mode" be sufficient? We would gain the same UX benefits of making it easier to debug programs, but then still be able to not generate the debug symbols.

It just feels a bit off to only be able to generate a "debug build", where your typical compiler toolchain always gives you that option (e.g. C/C++ compilers have a -g flag, cargo build {--release} for Rust, etc).

cc @PhilippGackstatter

plafer avatar May 23 '25 17:05 plafer

Technically, that option would still exist, it would just be handled by the Miden equivalent of strip.

I think either approach is fine though.

bitwalker avatar May 23 '25 17:05 bitwalker

Removing the option would simplify the internals of the assembler a bit - so, I think it is still preferred. Functionality wise, we could offer an option that first assembles a program and then strips debug info. From the user standpoint this could be equivalent to --release build.

bobbinth avatar May 23 '25 19:05 bobbinth

While working on this issue, I noticed a difficulty with stripping the decorators from a MastForest. That is, in the MastForestBuilder, the decorators on a node are considered when deciding whether 2 nodes are equal (with our MastNodeFingerprint). Therefore, when we strip the decorators, some nodes that were previously unequal become equal (if they're equal except for their decorators).

We should probably do #1776 before then - after which the AssemblyOp/source mapping information will no longer be considered in the equality check between nodes. However, we'll still have the subtle problem that a MastForest built with decorators + strip them out can result in a different MastForest if it was built directly without decorators if we're not careful. For example,

proc.f
  add
end

prof.g
  add debug.stack
end

If we build without decorators, both f and g will be compiled down to the same MastNodeId; but if we build with decorators, they will be compiled to different MastNodes, and (blindly) stripping decorators will result in a MastForest with 2 nodes instead of 1.

plafer avatar May 29 '25 21:05 plafer

Given the above, I think we should just switch the default to be debug_mode=true for release 0.15, and only fully remove debug_mode once we have a better idea of how to properly strip decorators.

plafer avatar Jun 02 '25 13:06 plafer

If we build without decorators, both f and g will be compiled down to the same MastNodeId; but if we build with decorators, they will be compiled to different MastNodes, and (blindly) stripping decorators will result in a MastForest with 2 nodes instead of 1.

Ah - good point! Yes, simply removing decorators could result in an odd state of the MastForest (it shouldn't cause any issues, but still will look somewhat unexpected). So, I agree, stripping out decorators would be a bit more involved and should be done as a separate task before this one.

bobbinth avatar Jun 04 '25 07:06 bobbinth