[PATCH] [AArch64] Condition codes AL and NV are invalid in the aliases that use inverted condition codes

Artyom Skrobov Artyom.Skrobov at arm.com
Thu Jun 5 04:58:45 PDT 2014


Hello Tim et al,

After committing the patch mentioned below, we realized that ARMARM forbids using the condition codes AL and NV in the five aliases that use inverted condition codes (CINC, CINV, CNEG, CSET, and CSETM), so our previous patch caused incorrect disassembly of the corresponding encodings (whereas they previously used to trigger an assertion failure when attempting to disassemble them).

Parsing the inv_ccode operand for these five aliases was already implemented with a custom handler in AArch64AsmParser::parseCondCode, so we're just adding the missing check into that function.

On the other hand, the auto-generated (by AsmWriterEmitter::EmitPrintAliasInstruction) handlers for matching aliases while disassembling, were restricted to:
* for a register parameter, check MI->getOperand(i).isReg()
**  for a "free" register parameter, check MRI.getRegClass(Target::XXXRegClassID).contains(MI->getOperand(i).getReg())
** for a "bound" register parameter, check MI->getOperand(i).getReg() == MI->getOperand(j).getReg())
** for a "fixed" register parameter, check MI->getOperand(i).getReg() == Target::XXX
* for a "fixed" immediate parameter, check MI->getOperand(i).isImm() && MI->getOperand(i).getImm() == XXX

There was no support for "immediate classes", which would let an alias match more than one but less than all possible immediate values; so this needed implementation at AsmWriterEmitter level.

Our new patch adds a new entry called MCOperandPredicate into class Operand, which may optionally hold a C++ snippet acting on an MCOp value. The C++ expression must evaluate to a boolean indicating whether the MCOp contains a valid value for the specific instance of Operand. inv_ccode is the first user of MCOperandPredicate, but this new facility should be generic enough to come useful in other backends as well.

The two other attached patches are much simpler. They address two other issues I've come across in TableGen while working on the patch for inv_ccode: one of them fixes the support for anonymous definitions inside a foreach block, and the other factors out the composition of a string fragment which was reused in each of the switch branches.


-----Original Message-----
From: Tim Northover [mailto:t.p.northover at gmail.com] 
Sent: 29 May 2014 11:49
To: Artyom Skrobov
Cc: llvm-commits
Subject: Re: [PATCH] [AArch64] Fix MC assertion failures with certain invalid instructions

Hi Artyom,

On 29 May 2014 11:36, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:
> We have identified two MC issues in the newly-merged AArch64 backend.
> These lead to assertion failures when handling certain invalid instructions.

For the inverted condition code patch, I think it'd be very good to
comment what instruction that might be interpreted as. Also, if it's
undefined it should have a "# CHECK: invalid instruction encoding"
line associated (otherwise LLVM could start disassembling it, or any
other instruction in that file, and we'd not notice). If not, it
should be tested in a different file.

The too few operands patch looks fine.

Cheers.

Tim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: AnonDefForeach.patch
Type: application/octet-stream
Size: 1647 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140605/146419af/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Refac_EmitPrintAlias.patch
Type: application/octet-stream
Size: 2642 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140605/146419af/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: inv_ccode.patch
Type: application/octet-stream
Size: 8791 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140605/146419af/attachment-0002.obj>


More information about the llvm-commits mailing list