Conversation
Read label names, and use the names when building the IR. Blocks and loops can hold names directly. `if`, `try` and `try_table` cannot, so for them the name is only a hint: we use it when a branch to the scope means we have to create a wrapper block anyhow, and otherwise drop it, so that reading these names never changes the shape of the IR.
cb552ac to
7ddcda1
Compare
Write the label subsection (id 3) next to the function and local names when debug info is enabled. Only explicit names are written: the ones that came from the text format, from the name section, or through the C API, and not the ones we generate ourselves. Functions therefore gain an `explicitLabelNames` set, filled by IRBuilder when a label comes from outside rather than from makeFresh(). Keeping this next to `localNames` and `debugLocations` rather than on the expressions themselves is how the rest of our debug info is stored, and it has two advantages: ExpressionAnalyzer never sees it, so two otherwise-identical blocks cannot stop comparing equal because of debug info; and since label names are unique within a function, it keeps identifying the right labels as optimizations replace the expressions that carry them.
7ddcda1 to
01f2737
Compare
| void writeExpression(Expression* curr); | ||
| void writeFunctions(); | ||
| void noteLabelNames(Function* func, | ||
| std::vector<std::pair<Index, Name>>& labelNames); |
There was a problem hiding this comment.
This is a helper, not one of the core write* methods, so perhaps let's move it to a less prominent place? (maybe around line 1585, the end of the class) Also, it could use a comment as to what it does and what the parameters mean.
| ;; CHECK-NEXT: (block $label | ||
| ;; CHECK-NEXT: (try $try1 | ||
| ;; CHECK-NEXT: (block $try1 | ||
| ;; CHECK-NEXT: (try |
There was a problem hiding this comment.
Why are there changes in this file? I don't seem to see it writing a binary, so the names section changes should not apply..?
There was a problem hiding this comment.
I have made some changes in the IRBuilder to better place labels. For try, we have the choice of putting the label on the wrapper block (as is done for if and try_table) or on the try itself. When there are only branches targeting it (no delegate or rethrow), I think it makes more sense to put the label on the wrapper block: that's why we now have (block $try1), and (br_if $try1) instead of (br_if $label), in this file. Since the IRBuilder is shared between the binary and the text parser, this impacts both.
I can remove this change from this PR and always put the label on the try if you prefer.
There was a problem hiding this comment.
I see, thanks. Please move from it from this PR, then, to keep things simple, and that other PR nice and focused. This is already a large (but useful!) change.
There was a problem hiding this comment.
I have removed the change from the PR.
| // end up needing a label anyhow. | ||
| auto name = getNextLabelName(); | ||
| auto result = builder.makeIf(Name(), getBlockType()); | ||
| builder.setScopeNameHint(name); |
There was a problem hiding this comment.
makeIf accepts a name, and at a glance, seems to use it as best it can. Why can we not pass in the name here directly, rather than sending Name() and then calling setScopeNameHint? (Should makeIf call that method..?)
This branch has not been deployed
2 participants
This is the only section of the Extended Name Section proposal which is currently not handled.