Before this commit, many passes in revng-c were skipping over
non-isolated functions.
Now revng-pipeline takes care of removing non-isolated functions so that
check can be omitted everywhere.
Implement a new beautify pass which simplifies away `SwitchBreakNode`s
that constitute the entire body of `case`s in `switch`es, that do not,
have a `default` case. In such situations indeed, the semantics is,
preserved by removing the `SwitchBreakNode`s.
We drop the assumption that each weaved `switch` must be nested inside
its related main `switch`, as a consequence of having generalized the
tiling algorithm in order to be able to emit a weaved `switch` as a AST
successors of the related main `switch`.
Various improvements to the debug graphs for `restructure-cfg` and
`beautify`:
- Normalize casing and syntax of debug graphs.
- Improve the graph folders name and layout.
- Implement `CFGDumper` and `ASTDumper` for when we need a serialization
with incremental indexes.
- Remove old and stale graph serializations.
Perform a complete rewrite of the `SwitchNode` tiling routine.
The tiling now works in the following way:
- When encountering a node which will produce a `SwitchNode` (either a
standard `switch` or a dispatcher `switch`), we look for the following
situations:
1) We have a node, a successor (case) of the `switch`, which in turn
is the successor of all the other successors (cases) of the
`switch`.
2) We have a node, not a successor (case) of the `switch`, which is
the successor of all the successors (cases) of the `switch`.
- If we find such candidate node, this node will be the fallthrough of
the `switch`. In addition, depending on whether the `switch` dominates
the candidate fallthrough, we can incorporate it as the immediate
successor of the `SwitchNode` we are building.
- There is currently an exception to the above, due to how we currently
handle weaved `switch`es. In such cases, we mandate that the weaved
`switch` is nested inside the main corresponding `switch`. For this
reason, we have a special casing handling the "all inlined but one"
situation in the new code, while this part could in theory be merged
in the common criterion below, at the cost of dropping the invariant
of the nesting of weaved switch`es.
Simplify `switch` cases that are simplified away during the dispatcher
`switch` inlining beautify.
Usually, we can simplify ast nodes by replacing them with `nullptr`s,
but due to how the `switch` cases work, we need to handle them in a
custom way.
We introduce the `InlineDispatcherSwitch` beautify pass. Its goal is to
try and inline the body of some of the `case`s of a exit dispatcher, in
place of the `SetNode` corresponding to that `case`, if this doesn't
introduce duplication in the code (i.e., a single `SetNode` for that
specific case value is present).
Additionally, if the inlining procedure is able to completely remove the
necessity of an exit dispatcher altogether, the pass removes it.
The pass is able to handle chains of weaved dispatcher `switch`es
referring to the same original dispatcher `switch`, by handling the
inline operation and the possible simplification level-wise.
The inlining procedure, cannot take place if a `SetNode` is contained in
the body of the case we are trying to inline, since this can possibly
break the semantics of the state variable of a loop, by placing a
`SetNode` in a more internal loop.
We introduce the PromoteCallNoReturn beautification pass. Its goal is to
restructure sequence of statements, in order to have `call`s to
`noreturn` functions as _inlined_ in the middle of the statement
sequence, and leave _non local control flow statements_ at the end of
that scope. E.g., we prefer:
```
if (cond)
call noreturnfunc();
return;
```
to
```
if (!cond)
return;
call noreturnfunc();
```
In order to do this, contextually, we restructure the routine computing
the `fallthrough` property, in order to be able to differentiate between
the _non local control flow statements_, a call to a `noreturn`
function, or a generic mix of the two (useful when combining results
from the two situations above).
The new analysis is also used in the `promoteNoFallThrough` promotion
pass.
Introduce the `DispatcherKind` attribute as a field in the
`BasicBlockNode` and `ASTNode` classes, in order to be able to
distinguish entry and exit dispatchers, and the related set nodes.
This commits reorders the beautifiers so that do-while loops are matched
before while loops.
This has the effect that loops that can be matched both as while and
do-while loops end up matched as do-while.
The consequence is that we generate C code with a layout that is much
more similar to assembly, and prevents duplicating code to recompute the
condition of the while.
Handle the situation where the whole dispatcher `switch` is simplified
after the `SimplifyDualSwitch` is run.
Enforce check that when a `SwitchBreakNode` is removed due to the
`SimplifyDualSwitch` it must be pertinent to the containing dispatcher
`switch`.
Move the `simplifyImplicitContinue` at the end of the beautify pass,
after all the pass that modify the anatomy of the ASTTree.
Before this change, the pass was run before some changes that moved some
`continue`s in positions where they are not considered implicit.
Added an assertion which checks that during the `noFallThrough`
promotion phase, no implicit `ContinueNode` is present.
The container used to store the associated `ExprNode`s that we need to
flip when performing a `HybridNot` simplification, is changed from a
`llvm::SmallVector` to `llvm:SmallSet`.
Indeed, during the collection of the `ExprNode`s pointing to a
`llvm::BasicBlock`, it should not be possible to find a single
`ExprNode` twice. We now assert this fact.
Improve the `fallThroughScope` computation, in order to handle calls to
`NoReturn` functions in the analysis, representing the fact that they
induce a `noFallthrough` scope (i.e., execution will not ever resume
after the call, and therefore we can later improve the nesting of the
code with the `promoteNoFallthrough` pass).
We introduce a simplification step, which looks for `switch`es that can
be reduced to simpler `if` statements.
Specifically, the logic is the following:
1) When we identify a `switch` statement composed by a single `case` and
a possible default, we transform it into an `if` with the `case` now
corresponding to the `then`, and the `default` corresponding to the
`else`, if present.
2) When we identify a `switch` statement composed by two `case`s, and no
`default` is present, we can promote it to an `if` with `then` `else`
branches.
Other key details:
- The promotion happens only if we can identify at least one of the
`case`s that have a single element in the `case` label. If this is not
the case, we do not promote one to RHS of the `if` condition.
- A new `CompareNode` class, inheriting from `ExprNode`, is created, in
order to represent the equality or inequality condition of an `IfNode`
instance that is the result of the promotion. This `CompareNode` can
represent for the LHS both an `llvm::Value` or the `loop_state_var`,
while it embeds the RHS constant which completes the comparison.
- We remove `SwitchBreak` AST nodes that may now appear as children of
an `if` node promoted from a `switch`.
- We introduce in the `CompareNode` the `weaved` concept. Indeed, if a
promotion of a weaved `switch` happens, we should avoid the
serialization of the instructions leading to the computation of the
condition of the original `switch`, because they have been already
emitted by the main related dominating `switch`.
We also introduce an additional simplification step, which takes care
of:
- Promoting `!(==)` to `(!=)` and `!(!=)` to `(==)`, if the inner
equal/not equal is represented via a `CompareNode`.
- Promoting `x == 0` to `!x` and `x != 0` to `x`.
To be able to correctly emit (or not) the instructions computing a
condition of an `IfNode`, we need to add the `EmittBB` flag, an
additional parameter to the `buildGHASTCondition` function, which
controls the emission of the statements of a basic block computing a
condition.
Consequently, the `IfNode` acquires a `IsWeaved` field, which is used to
mirror the property having the same name on `SwitchNode`. Being now
possible a promotion from a dual `SwitchNode` to an `IfNode`, we need to
represent this property on the `IfNode` too.
The `default` `case` is now a standard `case`, and it is identified only
by having the `label` set empty.
Therefore, a list of beautify and transformation actions now do not need
special casing in order to handle the `default` `case`, which is reached
during the standard iteration over the `case`s.
A special accessor is still necessary, in order to correctly emit the
`default` `case` in the backend.
Introduce implicit statements simplification phase, specifically:
- A implicit `return` simplification: `return` statements in `void`
type functions, which are not followed by any other scope, can be
omitted.
- A implicit `continue` simplification: `continue` statements whose
fallthrough leads directly to the end of the cycle scope (i.e., to
execute another iteration of the enclosing loop), can be omitted.
In order to avoid the printing of the implicit `return`, we need an
additional `emitReturn` parameter in the `emitBasicBlock` method of the
`CCodeGenerator` class.
When copying nodes from nested AST to the root one, we should take care
of overwriting the mapping between the old `OldCFGNode`, so that it now
points to the newer AST node representing it.
Now, also `break` and `continue` on the AST, when printed, have the ID
number of the `ASTNode`, and in the name they preserve the original
`CFGNode` one also.
Improve the logic of the first iteration outline:
1) The cloning of the nodes involved in the first iteration outlining,
is now performed with a series of DFS visits from all the late entry
nodes in the `MetaRegion`, instead of performing a blind clone of all
the nodes and removing those not needed.
2) Restoring the edges between the cloned nodes has been subject to
minor changes.
3) The outline nodes are now assigned to a `MetaRegion` using a new
logic, instead of blindly assigning them to the parent `MetaRegion`.
Remove old broken code that handled `Set` nodes as source of retreatings
when inserting an `entry` dispatcher. This situation should no longer be
the case, because a `Set` node should not be the `source` of a
retreating edge. We assert this situation.
We now explicitly store the retreatings edges, that will eventually be
connected to the `continue` nodes, so that we can explicitly handle
them, without resorting to iterating over the predecessors of the
`EntryNode`, which although correct is a fragile method with respect to
defective situations.
In addition, we check that after the restructuring, all the retreating
edges that we connect to `continue` nodes, do point to the `EntryNode`
before the transformation.
Before this commit, the code was using an old arcane algorithm that was
adding and removing dummy frontier nodes, and relying on the
DominatorTree.
This commit drops the DominatorTree altogether, drops the use of
frontier nodes, and rewrites the algorithm only using successors and
predecessors.
Now a node outside a metaregion M is included in the metaregion only if
all its predecessors are part of the metaregion, and none of them is
already in another metaregion that is a transitive parent of M.
Before this commit, the whole body of each loop was outlined and
duplicated, only to be removed later if not necessary.
Now we only do this if there is more than one entry to the loop, which
is the simplest condition to detect when first iteration outlining is
needed.
This commit drops some old broken code that was a leftover from when
MarkAssignments didn't inject calls to Assign, and that was poorly
migrated to Assign, causing it to be dead code.
This commit fixes a bug in the hasSideEffects function, used to evaluate
if an ExprNode has side effects.
Before this commit, when calling hasSideEffects on a NotNode, the
return value was true if the operand of NotNode **did not** have side
effects, which is wrong.
This commit changes the behaviour so that a NotNode has side effects if
and only if its only operand has side effects.
This commit fixes a bug in the hasSideEffects function, used to evaluate
if an ExprNode has side effects.
Before this commit, when calling hasSideEffects on an AndNode, the
return value was true if **both** the LHS and RHS of the AndNode had
side effects, which is wrong.
This commit changes the behaviour so that if **either** LHS **or** the
RHS of the And have any side effect, than hasSideEffects returns true
for the And.
Before this commit, the simplifyShortCircuit beautifier did not recur on
the then/else branch of an IfNode, basically stopping recursion very
early and potentially missing a lot of beautification opportunities.
Pietro Fezzardi