Add `--detect-abi-after-inline-ir-dir=<dir>` which writes one
`<fn>.ll` file per outlined stub immediately after the helper inlinining
has taken place.
Inline `revng_inline` helper calls into each outlined stub which is
consumed by `analyzeABI`, so the analysis can observe the reads and
writes the helper performs on the floating point registers.
The helper module is linked only once at the beginning of the pass, to
avoid double linking issues.
Split the `InlineHelpers` pass into the three phases:
- `link-helpers-to-inline` links the missing `revng_inline` helper
bodies from the `libtcg-helpers-to-inline-arch` module.
- `inline-helpers` inlines, in every function except those tagged
`Root` or `Helper`, the calls to `revng_inline` helpers whose
critical arguments on the call site are constant.
- `delete-helper-bodies` drops from the module the body of every
`revng_inline` function, leaving only the declaration.
The three passes live in a dedicated `revngInlineHelpers` library
(`lib/InlineHelpers/` + `include/revng/InlineHelpers/`), so that
libraries which do not depend on `revngFunctionIsolation` can use them
without creating cyclic dependencies.
The previous logic for detection of incompatible PHINodes was broken in
various different ways.
1. `return true` and `return false` were flipped, so it effectively did
the opposite of what it was expected to do
2. `std::numeri_limits<Value *>::max()` yielded 0, breaking the lookup.
This commit fixes the condition for incompatible PHINodes and adds a lot
of logging.
Before this commit, the code never put in the same group two PHINodes if
one of them had a non-phi User. This condition was wrong and was
unnecessarily preventing valig PHINodes to be put in the same group.
This caused the generation of a lot more local variables than necessary.
Before this commit we were replacing all uses of a PHI that were not
used by other PHIs with llvm::UndefValue. That was wrong. The
replacement is valid only if the use is used by a PHINode that is in the
same group. This commit fixes the condition for such a replacement to
take place.
This commit introduces `FixPointerSize`, a pipe changing the
`DataLayout` of module in order to have a pointer size identical to the
one of `targetABI()`.
The pipe also performs safety checks to ensure this does not corrupt
semantics.
Several components in both pipeline YAMLs are slated for removal.
Prepend a legacy prefix to their names, and to the matching c++ code, so
they are clearly distinguished from the new Clift-based pipeline until
they are dropped.
Because of how `StringAttr` comparison works, two attributes with
the same name can be added to one module. Forcing `StringAttr`
recreation from a raw string prevents that.
We used not to put CSVs in `declarations-only`, since `lift` is able to
create them. However, this led to have less CSVs than those we get
post-inlining (some are only used in helpers). This represents a problem
since sometimes we ignore non-existing registers.
The type emitter printed inter-field padding but never the padding
between the end of the last field and the declared size of the
struct.
Since every struct is emitted packed annotation, any struct with
trailing padding had a wrong sizeof in C.
This commit fixes the problem by always printing trailing padding in
structs, unless user have explicitly opted out setting `ExplicitPadding`
to off.
When importing descriptive info for a single function, the body walk
now intercepts clift::UseOp operations and follows the symbol
reference to find the target FunctionOp or GlobalVariableOp.
Before this commit, the per-function overload of importDescriptiveInfo
recorded the rename only for the current function (plus helpers and
globals at module level, but not via UseOp).
The deduplication logic makes it cheap and safe for a single FunctionOp
or GlobalVariableOp to be used in many UseOps without importing
descriptive info twice. The same holds if the FunctionOp is also visited
at module level and not just via UseOp, as is the case for
self-recursive functions.
This commit adds a set of module-level ops whose descriptive info has
already been recorded to SymbolRenamer, and short-circuit
recordFunctionOpName / recordGlobalVariableOpName when the same op
shows up again.
The set lives on SymbolRenamer (and not on the Importer) because a
single importDescriptiveInfo invocation may construct several Importer
instances: the per-function overload visits the current function with
one Importer, then each helper/global at module level with a fresh one,
and any callee reached through a clift::UseOp would similarly create
another. The deduplication logic must be shared across all of them so
that the same target is not recorded twice.
Behavior is unchanged for the current call sites (each module-level op
is visited at most once today); the deduplication logic prepares for
when the body walk will also reache sibling declarations through
clift::UseOp, which is necessary for importing descriptive info
properly.
Split visitFunctionOp into two parts:
1. a new recordGlobalVariableOpName helper, which records the name for
any GlobalVariableOp;
2 the existing visitGlobalVariableOp, that now delegates to the helper.
The helper exists so that, like recordFunctionOpName, it can be invoked
from contexts other than the top-level module walk, in particular, when
a clift::UseOp in a function body refers to a GlobalVariableOp.
Pure refactor: behavior is unchanged.
Split visitFunctionOp into two parts:
1. a new recordFunctionOpName helper, which records the name for any
FunctionOp without touching the Importer's CurrentFunction state;
2. the existing visitFunctionOp, which keeps the CurrentFunction
lifecycle and now delegates the recording to the new helper.
This makes the recording reusable from contexts where mutating
CurrentFunction is not appropriate e.g. when reaching a callee
declaration from a body walk via a clift::UseOp, which will need in a
later commit.
Pure refactor: behavior is unchanged.
Having manual indexing leads to having to touch all the following
entries when one is removed. It also makes adding new ones to
the middle of the list really awkward.
Instead of manual indexing, use a simple macro to come up with
the instance name on the fly.
Before this fix, triggering "comment" action *between* other parts,
for example, between arguments, would trigger it on the function
itself.
That behavior was confusing, so comment action is not allowed there
anymore.
Andrea Gussoni