[PATCH] D120958: [TableGen] Add support for variable length instruction in decoder generator
Min-Yih Hsu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 15 13:15:46 PDT 2022
myhsu added a comment.
In D120958#3382237 <https://reviews.llvm.org/D120958#3382237>, @0x59616e wrote:
> We can traverse the Segments of VarLenInst and use `CGIOperandList::OperandInfo::ParseOperandName` to get the operand number, and then add that Segments into the correspoding `OpInfo` according to the operand number.
>
> For example, if we find "dst.reg" and use `CGIOperandList::OperandInfo::ParseOperandName` and know "Oh, this is the third operand", then we can add this into the third `OpInfo`.
>
> In this way we can avoid overload `BitsInit` completely.
This approach sounds reasonable to me
================
Comment at: llvm/utils/TableGen/FixedLenDecoderEmitter.cpp:498
+ AllInstructions[Opcode].EncodingDef->getValue("SoftFail");
+ const BitsInit *SFBits = RV ? dyn_cast<BitsInit>(RV->getValue()) : nullptr;
+ for (unsigned i = 0; i < Bits.getNumBits(); ++i) {
----------------
0x59616e wrote:
> myhsu wrote:
> > why do you want to change these three lines? I don't think `RV` is used anywhere else.
> My understanding is: `SFBits` could be null pointer (take a look at the if statement below), which means `SoftFail` is not necessary. But using `getValueAsBitsInit` will trigger assertion if `SoftFail` can not be found.
The use of `getValueAsBitsInit` might be wrong but it's irrelevant to this patch. I think it's better to put this change into another patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120958/new/
https://reviews.llvm.org/D120958
More information about the llvm-commits
mailing list