Combine the StartBits, EndBits, and FieldVals vectors into a single
vector of a struct that contains all 3 pieces of information.
Instead of storing EndBits, we store NumBits since that's what the users
want.
I've removed the BitNo variable as it was easy to construct calculate
from StartBit. I've also removed Num in favor of Islands.size().
This reduces the amount of space needed for vectors of bit_value_t
and allows the user of memset.
Also reorder the enum values so BIT_FALSE is 0 and BIT_TRUE is 1.
Instead of returning the number of bytes emitted, just take the iterator
by reference so the increments in emitULEB128 will update the copy in
the caller.
Also pass the iterator by reference to emitNumToSkip so we don't need a
separate I += 3 in the caller.
Rename `indent` to `Indent` and `o` to `OS`.
Rename `Indentation` to `Indent`.
Remove unused argument from `emitPredicateMatch`.
Change `Indent` argument to `emitBinaryParser` to by value.
Add PrintError and family overload that accepts a print function. This
avoids constructing potentially long strings for passing into these
print functions.
In a previous PR #81716, a new decoder function was added to
llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp. During
code review it was suggested that, as most of the decoder functions were
very similar in structure, that they be refactored into a single,
templated function. I have added the refactored function, removed the
definitions of the replaced functions, and replaced the references to
the replaced functions in AArch64Disassembler.cpp and
llvm/lib/Target/AArch64/AArch64RegisterInfo.td. To reduce the number of
duplicate references in AArch64RegisterInfo.td, I have also made a small
change to llvm/utils/TableGen/DecoderEmitter.cpp.
1. Remove 'AllModes' and 'DefaultMode' suffixes for DecoderTables under
default HwMode.
2. Introduce a less aggressive suppression for HwMode DecoderTable, only
reduce necessary tables duplications. This allows encodings under
different HwModes to retain the original DecoderNamespace.
3. Change 'suppress-per-hwmode-duplicates' command option from bool type
to enum type, allowing users to choose what level of suppression to use.
Refactor of the llvm-tblgen source into:
- a "Basic" library, which contains the bare minimum utilities to build
`llvm-min-tablegen`
- a "Common" library which contains all of the helpers for TableGen
backends. Such helpers can be shared by more than one backend, and even
unit tested (e.g. CodeExpander is, maybe we can add more over time)
Fixes#80647
Many decodeULEB128/decodeSLEB128 users need to increment the pointer.
Add helpers to simplify this common pattern. We don't add `end` and
`error` parameters at present because many users don't need them.
Pull Request: https://github.com/llvm/llvm-project/pull/85739
The decoder emitter is showing some signs of age. This patch makes a few
kinds of clean-ups:
- Use ranged-for more widely, including using enumerate() for those
loops maintaining a loop index along with the items.
- Reduce the number of arguments to fieldFromInsn (removes an out
reference parameter: CodingStandards). The insn_t argument to insnWithID
can/should probably be removed soon too since modern C++ allows us to
return a local container without a copy.
- Use raw strings for the large emitted code segments. This enhances
both readability and modifiability.
DecoderEmitter and CodeEmitterGen perform repeated linear walks over the
entire instruction list. This patch eliminates two more such walks.
The eliminated traversals visit every instruction merely to determine
whether the target has variable length encodings. For a target with
variable length encodings, the original any_of will terminate quickly.
But all targets other than M68k use fixed length encodings and thus
any_of must visit the entire instruction list.
Currently the decoder and encoder emitters will crash if DefaultMode is
used within an EncodingByHwMode. As can be done today for
RegInfoByHwMode and ValueTypeByHwMode, this patch adds support for this
usage in EncodingByHwMode:
let EncodingInfos =
EncodingByHwMode<[ModeA, DefaultMode], [EncA, EncDefault]>;
Currently the DecoderEmitter spends a fair amount of cycles performing
repeated linear walks over the entire instruction list. This patch
eliminates one such walk during HwMode collection for EncodingInfos.
The eliminated traversal visits every instruction and then every
EncodingInfos entry for that instruction merely to collect all
referenced HwModes. That information already happens to be present in
the HwModeSelects created during the one-time construction of
CodeGenHwModes. We instead traverse the HwModeSelects, collecting each
one referenced as an encoding select. This set is a small constant in
size and does not generally grow with the size of the instruction set.
Currently, for per-HwMode encoding/decoding, those instructions that do
not have a HwMode override are duplicated into the decoder tables for
all HwModes. This includes inducing multiple tables for instructions
that are otherwise unrelated (e.g., different namespace with no
overrides at all).
This patch adds support to suppress instruction and table duplicates.
TableGen option "-gen-disassembler --suppress-per-hwmode-duplicates"
enables the suppression (off by default).
For one downstream backend with a complicated ISA and major
cross-generation encoding differences, this eliminates ~32000 duplicate
table entries at the time of this patch.
There are legitimate reasons to suppress or not suppress duplicates. If
there are relatively few non-overridden related instructions, it can be
convenient to pull them into the per-mode tables (only need to decode
the per-mode tables, slightly simpler decode function in disassembler).
On the other hand, in some backends, the opposite is true or the size is
too large to tolerate any duplication in the first place. We let the
user decide which makes sense.
This is currently off by default, though there is no reason it couldn't
be enabled by default. Any existing backends downstream using the
per-HwMode feature will function as before. Turning on the feature
requires minor modifications to their disassembler due to more/less
tables and naming.
Today, if any instruction uses EncodingInfos/EncodingByHwMode to
override the default encoding, the opcode field of the decoder table is
generated incorrectly. This causes failed disassemblies and other
problems.
Specifically, the main correctness issue is that the EncodingID is
inadvertently stored in the table rather than the actual opcode. This is
caused by having set up the IndexOfInstruction map incorrectly during
the loop to populate NumberedEncodings-- which is then propagated around
when OpcMap is set up with a bad EncodingIDAndOpcode.
Instead, do away with IndexOfInstruction altogether and use opcode value
queried from CodeGenTarget::getInstrIntValue to set up OpcMap. This
itself exposed another problem where emitTable was using the decoded
opcode to index into NumberedEncodings. Instead pass in the
EncodingIDAndOpcode vector, and create the reverse mapping from Opcode
to EncodingID, which is then used to index NumberedEncodings.
This problem is not currently exposed upstream since no in-tree targets
yet use the per-HwMode feature. It does show up in at least two
downstream targets.
This patch removes a couple of redundant buffer copies in emitTable for
setting up calls to decodeULEB128. Instead, provide the Table.data
buffer directly to the calls-- where decodeULEB128 does its own buffer
overflow checking.
Factor out 7 explicit loops to emit ULEB128 bytes into emitULEB128. Also
factor out 4 copies of 24-bit numtoskip emission into emitNumToSkip.
The functionality is already covered by existing unit tests and by
virtue of most of the in-tree back-ends exercising the decoder emitter.
Both OPC_ExtractField and OPC_CheckField are currently defined to take
an unsigned 8-bit start value. On some architectures with long
instruction words, this value can silently overflow, resulting in a bad
decoder table.
This patch changes each to take a ULE128B-encoded start value instead.
Additionally, a range assertion is added for the 8-bit length to
prominently notify a user in case that field ever overflows.
This problem isn't currently exposed upstream since all in-tree targets
use small instruction words (i.e., bitwidth <= 64 bits). It does show up
in at least one downstream target with instructions > 64 bits long.
Co-authored-by: Jason Eckhardt <jeckhardt@nvidia.com>
SubtargetFeature.h is currently part of MC while it doesn't depend on
anything in MC. Since some LLVM components might have the need to work
with target features without necessarily needing MC, it might be
worthwhile to move SubtargetFeature.h to a different location. This will
reduce the dependencies of said components.
Note that I choose TargetParser as the destination because that's where
Triple lives and SubtargetFeatures feels related to that.
This issues came up during a JITLink review (D149522). JITLink would
like to avoid a dependency on MC while still needing to store target
features.
Reviewed By: MaskRay, arsenm
Differential Revision: https://reviews.llvm.org/D150549
After the work in a538d1f13a13 5351878ba196 372240dfe3d5, and
subsequently cleanup of all the in-tree targets, we can now delete the
support for positional operand matching!
This removes three options which could previously be set in a
Target's "InstrInfo" tablegen definition:
- useDeprecatedPositionallyEncodedOperands
- decodePositionallyEncodedOperands
- noNamedPositionallyEncodedOperands
(Also announced at https://discourse.llvm.org/t/tablegen-deleting-deprecated-positional-instruction-operand-matching-support/68524)
Differential Revision: https://reviews.llvm.org/D144210
Just like the encoder directive for variable-length instructions, this
patch adds a new decoder directive to allow custom decoder function on
an operand.
Right now, due to the design of DecoderEmitter each operand can only
have a single custom decoder in a given instruction.
Differential Revision: https://reviews.llvm.org/D142079
Commit a538d1f13a13 first added support for named sub-operands in
CodeEmitterGen. We now add a few more features to that, enabling
further target cleanups.
1. Adds support for handling an EncoderMethod in a sub-operand in
CodeEmitterGen. Previously, the specified encoder of a sub-operand was
ignored, and only the default used.
2. Adds support for sub-operands in DecoderEmitter, along with support
for tied sub-operands.
The changes to the decoder required a few minor tweaks to a few
targets, where existing brokeness was exposed. In order to keep this
patch small, I left FIXMEs which will be addressed in upcoming
patches. (Except MIPS16, since its object file emission/decoding is
totally broken).
Differential Revision: https://reviews.llvm.org/D137653
Currently the DecoderEmitter constructor takes a bunch of string
parameters containing bits of code to interpolate.
However, there's only two ways it can be called. The one used for most
targets which doesn't handle the SoftFail DecoderStatus (not a
problem, because they don't use SoftFail). The other mode, which is
used for ARM/AArch64, does handle SoftFail, but requires an externally
defined helper function in those targets.
This is unnecessary complication; remove the parameters, and unify
onto a single version which does support SoftFail, defining the helper
itself.
Summary:
The existing undefined-bitfield-to-operand matching behavior is very
hard to understand, due to the combination of positional and named
matching. This can make it difficult to track down a bug in a target's
instruction definitions.
Over the last decade, folks have tried to work-around this in various
ways, but it's time to finally ditch the positional matching. With
https://reviews.llvm.org/D131003, there are no longer cases that
_require_ positional matching, and it's time to start removing usage
and support for it.
Therefore: add a (default-false) option, and set it to true only in
those targets that require positional matching today. Subsequent
changes will start cleaning up additional in-tree targets.
NOTE TO OUT OF TREE TARGET MAINTAINERS:
If this change breaks your build, you may restore the previous
behavior simply by adding:
let useDeprecatedPositionallyEncodedOperands = 1;
to your target's InstrInfo tablegen definition. However, this is
temporary -- the option will be removed in the future.
If your target does not set 'decodePositionallyEncodedOperands', you
may thus start migrating to named operands. However, if you _do_
currently set that option, I recommend waiting until a subsequent
change lands, which adds decoder support for named sub-operands.
Differential Revision: https://reviews.llvm.org/D134073
These names can then be matched by name against 'bits' fields in a
record, to populate an instruction's encoding.
This does _not_ yet change DecoderEmitter to allow by-name matching of
sub-operands. Unlike the encoder, the decoder already defaulted to not
supporting positional matching, and backends had workarounds in place
for the missing decoding support.
Additionally, use this new capability to allow the ARM and AArch64
backends not to require any positional operand matching.
Differential Revision: https://reviews.llvm.org/D131003
