[llvm] r309949 - [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 3 17:26:58 PDT 2017
reverted in r310008
On Thu, Aug 3, 2017 at 5:20 PM, Vitaly Buka <vitalybuka at google.com> wrote:
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6924/steps/check-llvm%20ubsan/logs/stdio
>
>
> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.
> FAIL: LLVM :: MC/AMDGPU/exp.s (14527 of 21592)
> ******************** TEST 'LLVM :: MC/AMDGPU/exp.s' FAILED ********************
> Script:
> --
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/./bin/llvm-mc -arch=amdgcn -show-encoding /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/test/MC/AMDGPU/exp.s | /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/./bin/FileCheck -check-prefix=SI /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/test/MC/AMDGPU/exp.s
> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/./bin/llvm-mc -arch=amdgcn -mcpu=tonga -show-encoding /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/test/MC/AMDGPU/exp.s | /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/./bin/FileCheck -check-prefix=VI /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/test/MC/AMDGPU/exp.s
> --
> Exit Code: 2
>
> Command Output (stderr):
> --
> lib/Target/AMDGPU/AMDGPUGenAsmMatcher.inc:9073:33: runtime error: index -1 out of bounds for type 'unsigned int [12]'
> #0 0x4e1272 in (anonymous namespace)::AMDGPUAsmParser::convertToMCInst(unsigned int, llvm::MCInst&, unsigned int, llvm::SmallVectorImpl<std::unique_ptr<llvm::MCParsedAsmOperand, std::default_delete<llvm::MCParsedAsmOperand> > > const&, llvm::SmallBitVector const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/lib/Target/AMDGPU/AMDGPUGenAsmMatcher.inc:9073:33
> #1 0x4c7230 in (anonymous namespace)::AMDGPUAsmParser::MatchInstructionImpl(llvm::SmallVectorImpl<std::unique_ptr<llvm::MCParsedAsmOperand, std::default_delete<llvm::MCParsedAsmOperand> > > const&, llvm::MCInst&, unsigned long&, bool, unsigned int) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/lib/Target/AMDGPU/AMDGPUGenAsmMatcher.inc:19873:5
> #2 0x4b55b8 in (anonymous namespace)::AMDGPUAsmParser::MatchAndEmitInstruction(llvm::SMLoc, unsigned int&, llvm::SmallVectorImpl<std::unique_ptr<llvm::MCParsedAsmOperand, std::default_delete<llvm::MCParsedAsmOperand> > >&, llvm::MCStreamer&, unsigned long&, bool) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:2222:14
> #3 0x8f37ad in (anonymous namespace)::AsmParser::parseStatement((anonymous namespace)::ParseStatementInfo&, llvm::MCAsmParserSemaCallback*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/MC/MCParser/AsmParser.cpp:2224:27
> #4 0x8eb2c8 in (anonymous namespace)::AsmParser::Run(bool, bool) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/MC/MCParser/AsmParser.cpp:883:10
> #5 0x454d70 in AssembleInput(char const*, llvm::Target const*, llvm::SourceMgr&, llvm::MCContext&, llvm::MCStreamer&, llvm::MCAsmInfo&, llvm::MCSubtargetInfo&, llvm::MCInstrInfo&, llvm::MCTargetOptions&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-mc/llvm-mc.cpp:425:21
> #6 0x453529 in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-mc/llvm-mc.cpp:608:11
> #7 0x7f0e4bcf382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
> #8 0x42d138 in _start (/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llvm-mc+0x42d138)
>
> FileCheck error: '-' is empty.
> FileCheck command line: /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/./bin/FileCheck -check-prefix=SI /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/test/MC/AMDGPU/exp.s
>
>
> On Thu, Aug 3, 2017 at 8:40 AM, Nirav Dave via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: niravd
>> Date: Thu Aug 3 08:40:21 2017
>> New Revision: 309949
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=309949&view=rev
>> Log:
>> [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is
>> true
>>
>> 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
>>
>> Subscribers: llvm-commits
>>
>> Differential Revision: https://reviews.llvm.org/D35998
>>
>> Modified:
>> llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
>>
>> Modified: llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGe
>> n/AsmMatcherEmitter.cpp?rev=309949&r1=309948&r2=309949&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp (original)
>> +++ llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp Thu Aug 3 08:40:21
>> 2017
>> @@ -1847,13 +1847,30 @@ static void emitConvertFuncs(CodeGenTarg
>> 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) << "];\n";
>> + CvtOS << " assert(OptionalOperandsMask.size() == " <<
>> (MaxNumOperands)
>> + << ");\n";
>> + CvtOS << " for (unsigned i = 0, NumDefaults = 0; i < " <<
>> (MaxNumOperands)
>> + << "; ++i) {\n";
>> + CvtOS << " DefaultsOffset[i] = 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";
>> + // OpIdx has different semantics for Tied operands and the rest of
>> the
>> + // operands. For Tied it is the index in the Inst, therefore we use
>> it
>> + // directly. For the rest of the operands, we need to account for the
>> + // offset.
>> + CvtOS << " OpIdx = *(p + 1);\n";
>> + CvtOS << " OpIdx -= (*p != CVT_Tied) ? DefaultsOffset[*(p + 1) -
>> 1] : "
>> + "0;\n";
>> } else {
>> CvtOS << " OpIdx = *(p + 1);\n";
>> }
>> @@ -1988,7 +2005,6 @@ static void emitConvertFuncs(CodeGenTarg
>> << " " << Op.Class->DefaultMethod << "()"
>> << "->" << Op.Class->RenderMethod << "(Inst, "
>> << OpInfo.MINumOperands << ");\n"
>> - << " ++NumDefaults;\n"
>> << " } else {\n"
>> << " static_cast<" << TargetOperandClass
>> << "&>(*Operands[OpIdx])." << Op.Class->RenderMethod
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170803/55d126fc/attachment.html>
More information about the llvm-commits
mailing list