Skip to content

Commit

Permalink
[TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands…
Browse files Browse the repository at this point in the history
… is true

Relanding after fixing UB issue with DefaultOffsets.

Consider the following instruction: "inst.eq $dst, $src" where ".eq"
is an optional flag operand.  The $src and $dst operands are
registers.  If we parse the instruction "inst r0, r1", the flag is not
present and it will be marked in the "OptionalOperandsMask" variable.
After the matching is complete we call the "convertToMCInst" method.

The current implementation works only if the optional operands are at
the end of the array.  The "Operands" array looks like [token:"inst",
reg:r0, reg:r1].  The first operand that must be added to the MCInst
is the destination, the r0 register.  The "OpIdx" (in the Operands
array) for this register is 2.  However, since the flag is not present
in the Operands, the actual index for r0 should be 1.  The flag is not
present since we rely on the default value.

This patch removes the "NumDefaults" variable and replaces it with an
array (DefaultsOffset).  This array contains an index for each operand
(excluding the mnemonic).  At each index, the array contains the
number of optional operands that should be subtracted.  For the
previous example, this array looks like this: [0, 1, 1].  When we need
to access the r0 register, we compute its index as 2 -
DefaultsOffset[1] = 1.

Patch by Alexandru Guduleasa!

Reviewers: SamWot, nhaustov, niravd

Reviewed By: niravd

Subscribers: vitalybuka, llvm-commits

Differential Revision: https://reviews.llvm.org/D35998

llvm-svn: 310254
  • Loading branch information
niravhdave committed Aug 7, 2017
1 parent ec4ea99 commit b2f3fad
Showing 1 changed file with 14 additions and 3 deletions.
17 changes: 14 additions & 3 deletions llvm/utils/TableGen/AsmMatcherEmitter.cpp
Expand Up @@ -1847,13 +1847,25 @@ static void emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
CvtOS << " assert(Kind < CVT_NUM_SIGNATURES && \"Invalid signature!\");\n";
CvtOS << " const uint8_t *Converter = ConversionTable[Kind];\n";
if (HasOptionalOperands) {
CvtOS << " unsigned NumDefaults = 0;\n";
size_t MaxNumOperands = 0;
for (const auto &MI : Infos) {
MaxNumOperands = std::max(MaxNumOperands, MI->AsmOperands.size());
}
CvtOS << " unsigned DefaultsOffset[" << (MaxNumOperands + 1)
<< "] = { 0 };\n";
CvtOS << " assert(OptionalOperandsMask.size() == " << (MaxNumOperands)
<< ");\n";
CvtOS << " for (unsigned i = 0, NumDefaults = 0; i < " << (MaxNumOperands)
<< "; ++i) {\n";
CvtOS << " DefaultsOffset[i + 1] = NumDefaults;\n";
CvtOS << " NumDefaults += (OptionalOperandsMask[i] ? 1 : 0);\n";
CvtOS << " }\n";
}
CvtOS << " unsigned OpIdx;\n";
CvtOS << " Inst.setOpcode(Opcode);\n";
CvtOS << " for (const uint8_t *p = Converter; *p; p+= 2) {\n";
if (HasOptionalOperands) {
CvtOS << " OpIdx = *(p + 1) - NumDefaults;\n";
CvtOS << " OpIdx = *(p + 1) - DefaultsOffset[*(p + 1)];\n";
} else {
CvtOS << " OpIdx = *(p + 1);\n";
}
Expand Down Expand Up @@ -1988,7 +2000,6 @@ static void emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
<< " " << Op.Class->DefaultMethod << "()"
<< "->" << Op.Class->RenderMethod << "(Inst, "
<< OpInfo.MINumOperands << ");\n"
<< " ++NumDefaults;\n"
<< " } else {\n"
<< " static_cast<" << TargetOperandClass
<< "&>(*Operands[OpIdx])." << Op.Class->RenderMethod
Expand Down

0 comments on commit b2f3fad

Please sign in to comment.