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 de-serializing, MLIR pre-populates the value outside of our
control. As such, we cannot assert that it's always empty. So,
the least we can do is assert that value is the same.
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.
Pietro Fezzardi