fix!: Allow Const and FuncDecl as children of Modules, Dataflow Parents, and CFG nodes

[doug-q]

We take the opportunity to fix a bug where the promise to ignore duplicate calls to emit_func was not upheld, and to clean up the now unnecessary Emission.

Fixes #4 , although we go further than required by this issue.

BREAKING CHANGE: rename emit_global to emit_func. Replace Emission type with FatNode<FuncDefn>.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 90.52632% with 9 lines in your changes missing coverage. Please review.

Project coverage is 83.12%. Comparing base (8dcba0f) to head (cbf3b74).

Files	Patch %	Lines
src/emit.rs	63.63%	2 Missing and 2 partials ⚠️
src/emit/ops/cfg.rs	82.35%	1 Missing and 2 partials ⚠️
src/emit/ops.rs	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
+ Coverage   82.19%   83.12%   +0.93%     
==========================================
  Files          16       16              
  Lines        2190     2234      +44     
  Branches     2190     2234      +44     
==========================================
+ Hits         1800     1857      +57     
+ Misses        258      246      -12     
+ Partials      132      131       -1     

Flag	Coverage Δ
rust	83.12% <90.52%> (+0.93%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[croyzor]

Any idea why we end up with all these redundant store and load cycles?

[doug-q]

Yes, we make no attempt to avoid them. Before we emit a node we load a value per input wire, then after we emit a node we store one value per output wire. Sometimes (for example Exit nodes) we even store and load to intermediate allocas that do not correspond to wires in the graph.

mem2reg removes these. There are(should be) no difficult cases because the value in the alloca is always unique.

We could potentially avoid emitting (most of) them, but this would make things more complicated.

[croyzor]

Thanks! 😄

[croyzor]

Any idea what's going on here? Just for my personal curiosity

[doug-q]

Yes, this corresponds to the let control = builder.add_load_value(Value::unary_unit_sum()); line. The out wire of this is of type "unary sum with one variant", which we encode in llvm as { {} }: a struct containing an empty struct. Normally Sum types also have an i32 tag, but when num_variants <= 1, we omit the tag. When we generate the code for the following switch, we "read the tag", which for this case(num_variants <= 1), just creates a value with constant value 0.

Obviously this looks dumb, it is the result of lowering the ops in a uniform way, with the goal of making the code here simple. The cost is that you generate dumb code. We do expect LLVM to be able to clean this up easily. In this case "dce" (dead-code-elimination) will delete %1 as it's unused and side-effect-free.

[croyzor]

Thanks Doug, makes perfect sense!

[doug-q]

Hmm, that's not quite the right explanation. When we switch at the end of a DataFlowBlock we must write to the RowMailBox of the target block. Note that this write has to happen AFTER the switch, because we only do it on the taken edge. When we write to that RowMailBox we first write each value in the Sum (which has a known variant tag because we are after the switch), then we write each of the "other inputs" (of the Output node in the Block we just left). So we extractvalue the struct of our variant from the struct representing the Sum. Then for each field in the struct, we extractvalue that and write it to the mail box. In this case, the struct of our variant is {} i.e. empty and there are no "other inputs", so the only thing we do is extractvalue the struct of our variant, which is what this line is.