Clone empty blocks before def-use analysis #3653
Conversation
Signed-off-by: Mihai Budiu <[email protected]>
|
@fruffy this fails in testgen with the following error: |
|
It looks like this also fixes #3620. |
|
If it fixes #3620 it is a coincidence. That one I still have to work on. |
Signed-off-by: Mihai Budiu <[email protected]>
We can leave the issue open then, but I do not have a reproducing test for this right now. Either way, it must be removed from the xfails here. |
| class SimplifyDefUse : public PassManager { | ||
| class Cloner : public ClonePathExpressions { | ||
| public: | ||
| Cloner() { setName("Cloner"); } | ||
| const IR::Node* postorder(IR::EmptyStatement* stat) override { | ||
| // You cannot clone an empty statement, since | ||
| // the visitor claims it's equal to the original one. | ||
| // So we cheat and make an empty block. | ||
| // So we convert it to an empty block. | ||
| return new IR::BlockStatement(stat->srcInfo); |
There was a problem hiding this comment.
Can you not directly produce a block statement with an annotation here? I do not quite understand some of the tricks that are being used here.
There was a problem hiding this comment.
I do produce an empty block, but no annotation is needed here.
There is really no trick, except I need each statement to have its own IR node.
| void clear() { | ||
| // This ensures that a clear map is never up-to-date, | ||
| // since the 'fake' node cannot appear in a program. | ||
| program = fake; |
There was a problem hiding this comment.
What is the purpose of this hack? To force a refresh of the program map every time?
There was a problem hiding this comment.
I would not call it a hack.
Each map stores the program that it was generated on. If the program hasn't changed, the map is good.
But an empty map is never up-to-date.
Signed-off-by: Mihai Budiu <[email protected]>
mihaibudiu
requested review from
pkotikal,
vhavel and
jnfoster
as code owners
It should be easy to create a reproduction by having a metadata and a header structure with a common field name. |
Signed-off-by: Mihai Budiu <[email protected]>
Signed-off-by: Mihai Budiu <[email protected]>
Signed-off-by: Mihai Budiu [email protected]
Fixes #3650
This was harder to fix than I expected. The visitor infra is too smart for its own good: it wouldn't duplicate empty blocks since it realizes they are identical. So you have to add annotations to empty blocks to make then different, then remove them again.
This also uncovered a bug in the frontend where RemoveUnusedDeclarations was called instead of RemoveAllUnusedDeclarations. That won't happen again, since I made the constructor private.