Using VEX_4V may not be the right fix since Intel guide says "Operand 3:
VEX.vvvv"
I was talking about this section of code:
void RecognizableInstr::handleOperand(
...
while (operandMapping[operandIndex] != operandIndex) {
Spec->operands[operandIndex].encoding = ENCODING_DUP;
Spec->operands[operandIndex].type (OperandType)(TYPE_DUP0 +
operandMapping[operandIndex]);
++operandIndex;
}
…
}
void RecognizableInstr::emitInstructionSpecifier(DisassemblerTables &tables)
{
...
case X86Local::MRMSrcMem:
// Operand 1 is a register operand in the Reg/Opcode field.
// Operand 2 is a memory operand (possibly SIB-extended)
// - In AVX, there is a register operand in the VEX.vvvv field here -
// Operand 3 (optional) is an immediate.
...
HANDLE_OPERAND(roRegister)
if (HasVEX_4VPrefix)
// FIXME: In AVX, the register below becomes the one encoded
// in ModRMVEX and the one above the one in the VEX.VVVV field
HANDLE_OPERAND(vvvvRegister)
if (HasMemOp4Prefix)
HANDLE_OPERAND(immediate)
HANDLE_OPERAND(memory)
if (HasVEX_4VOp3Prefix)
HANDLE_OPERAND(vvvvRegister)
…
}
For GATHER with 5 operands (dst, mask_wb, src1, mem, mask), the operandMapping
is "0 to 0, 1 to 1, 2 to 0, 3 to 3, 4 to 1", operand 2 is tied to
operand 0, operand 4 is tied to operand 1.
So operand 2 and 4 (src1, mask) are treated as DUP, and the physical operands
are 0,1,3(dst, mask_wb, mem), while MRSrcMem assumes Reg, Mem, Reg.vvvv if
HasVEX_4VOp3Prefix is true.
Same situation happens in X86MCCodeEmitter.cpp, where TIED_TO for operand 2 is
operand 0. We can only increment CurOp once for operand 2, since TIED_TO of
operand 1 is -1.
What we really want is to reverse the direction of TIED_TO for mask and mask_wb.
We can probably hack this to handle the special case of two tied-to operands, if
we have 2 tied-to operands, handle operand vvvvRegister first, then handle
memory operand in MRMSrcMem.
In X86MCCodeEmitter.cpp, we increase CurOp twice if we have 2 tied-to operands.
But it is kind of ugly.
+ // FIXME: Handle the special case for GATHER:
+ // For GATHER with 5 operands (dst, mask_wb, src1, mem, mask), src1 is tied
+ // to dst and mask is tied to mask_wb. The operandMapping is "0 to 0,
+ // 1 to 1, 2 to 0, 3 to 3, 4 to 1". The 2nd physical operand is
mask_wb, and
+ // it is before mem, so we need to explicitly handle vvvvRegister first.
+ if (HasVEX_4VOp3Prefix && tiedOperandsCount >= 2)
+ HANDLE_OPERAND(vvvvRegister)
+
Thanks,
Manman
On Jul 9, 2012, at 11:47 PM, Craig Topper wrote:
> I don't think changing to VEX_4VOp3 to VEX_4V is the right fix. I think
the fix is to increment CurOp twice at the start for these instructions so that
only the input operands are used for encoding.
>
> Also, I just submitted a patch to revert the operand order for these
instructions in the assembler/disassembler. Destination register should appear
on the right and the mask should appear on the left as we use AT&T syntax by
default. It will probably conflict with your updates here.
>
> On Mon, Jul 9, 2012 at 11:24 PM, Manman Ren <mren at apple.com>
wrote:
>
> Yes, there is an easy way to fix this.
> MRMSrcMem assumes register, memory, vvvv register if VEX_4VOp3 is true and
assumes register, vvvv register, memory if VEX_4V is true.
>
> I just need to change the flag from VEX_4VOp3 to VEX_4V. There are a few
places where we assume only the 2nd operand can be tied-to:
> Desc->getOperandConstraint(1, MCOI::TIED_TO) != -1 (hard-coded index 1)
> I will fix those to handle this instruction.
>
> Thanks,
> Manman
>
> On Jul 9, 2012, at 10:07 PM, Evan Cheng wrote:
>
> >
> >
> > On Jul 9, 2012, at 4:15 PM, Manman Ren <mren at apple.com>
wrote:
> >
> >>
> >> I need to implement an instruction which has 2 read-write
registers, so I added
> >> let Constraints = "$src1 = $dst, $mask = $mask_wb" in {
> >> ...
> >> def rm : AVX28I<opc, MRMSrcMem, (outs VR128:$dst,
VR128:$mask_wb),
> >> (ins VR128:$src1, v128mem:$src2, VR128:$mask),
> >> ...
> >> }
> >> There is a problem since MRMSrcMem assumes the 2nd physical
operand is a memory operand.
> >> See the section about MRMSrcMem in
RecognizableInstr::emitInstructionSpecifier.
> >
> > Can this be fixed?
> >
> > Evan
> >
> >> And the above gives us $dst, $mask_wb, $src1, $mem, $mask, and
$mask_wb is the second physical operand.
> >>
> >> I thought about using "$mask_wb = $mask", but it breaks
the assumption of TIED_TO LhsIdx > RhsIdx.
> >> Is adding another addressing mode a good idea?
> >>
> >> Any pointer is appreciated.
> >> Thanks,
> >> Manman
> >> _______________________________________________
> >> LLVM Developers mailing list
> >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>
> --
> ~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120710/37acfd7f/attachment.html>