<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div>Should we just fix the TIED_TO constraints to support "$mask_wb = $mask"? :)<div><br></div><div>Manman</div><div><br><div><div>On Jul 11, 2012, at 12:01 AM, Craig Topper wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">I was playing around with this myself this evening. Here's my attempt at a patch. I've merged in some stuff from your patch as well. I've attempted to fix X86RecognizableInstr.cpp in a different way.<br><br><div class="gmail_quote">
On Tue, Jul 10, 2012 at 11:10 PM, Manman Ren <span dir="ltr"><<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><br></div>GATHER instructions VGATHERxx take 3 operands: op1 is a register operand, op2 is a memory operand, op3 is a register operand.<div>Both op1 and op3 are read-write, after execution of the instruction, both are updated.</div>
<div><br></div><div>There are issues related to handling two TIED_TO constraints and how to use TIED_TO constraints to correctly encode/decode VGATHERxx.</div><div>The discussion at llvmdev is appended to the end of this message.</div>
<div><br></div><div>Please review and provide feedback.</div><div><br></div><div>Thanks,</div><div>Manman</div><div><div><div><br></div><div></div></div></div></div>
<br><div style="word-wrap:break-word"><div><div><div></div><div>Begin forwarded message:</div><br><blockquote type="cite"><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><span><b>From: </b></span><span style="font-family:'Helvetica';font-size:medium">Manman Ren <<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>><br>
</span></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><span><b>Subject: </b></span><span style="font-family:'Helvetica';font-size:medium"><b>Re: [LLVMdev] question on table gen TIED_TO constraint</b><br>
</span></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><span><b>Date: </b></span><span style="font-family:'Helvetica';font-size:medium">July 10, 2012 3:52:53 PM PDT<br></span></div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><span><b>To: </b></span><span style="font-family:'Helvetica';font-size:medium">Craig Topper <<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>><br>
</span></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><span><b>Cc: </b></span><span style="font-family:'Helvetica';font-size:medium">Evan Cheng <<a href="mailto:evan.cheng@apple.com" target="_blank">evan.cheng@apple.com</a>>, "<a href="mailto:llvmdev@cs.uiuc.edu" target="_blank">llvmdev@cs.uiuc.edu</a>" <<a href="mailto:llvmdev@cs.uiuc.edu" target="_blank">llvmdev@cs.uiuc.edu</a>><br>
</span></div><br><div style="word-wrap:break-word"><div><br></div><div>Using VEX_4V may not be the right fix since Intel guide says "Operand 3: VEX.vvvv"</div><div><br></div><div>I was talking about this section of code:</div>
<div>void RecognizableInstr::handleOperand(</div><div> ...</div><div><div> while (operandMapping[operandIndex] != operandIndex) {</div><div> Spec->operands[operandIndex].encoding = ENCODING_DUP;</div><div> Spec->operands[operandIndex].type =</div>
<div> (OperandType)(TYPE_DUP0 + operandMapping[operandIndex]);</div><div> ++operandIndex;</div><div> }</div></div><div> …</div><div>}</div><div><br></div><div>void RecognizableInstr::emitInstructionSpecifier(DisassemblerTables &tables) {</div>
<div>...</div><div><div> case X86Local::MRMSrcMem:</div><div> // Operand 1 is a register operand in the Reg/Opcode field.</div><div> // Operand 2 is a memory operand (possibly SIB-extended)</div><div> // - In AVX, there is a register operand in the VEX.vvvv field here -</div>
<div> // Operand 3 (optional) is an immediate.</div><div> ...</div><div> HANDLE_OPERAND(roRegister)</div><div><br></div><div> if (HasVEX_4VPrefix)</div><div> // FIXME: In AVX, the register below becomes the one encoded</div>
<div> // in ModRMVEX and the one above the one in the VEX.VVVV field</div><div> HANDLE_OPERAND(vvvvRegister)</div><div><br></div><div> if (HasMemOp4Prefix)</div><div> HANDLE_OPERAND(immediate)</div><div>
<br></div><div> HANDLE_OPERAND(memory)</div><div><br></div><div> if (HasVEX_4VOp3Prefix)</div><div> HANDLE_OPERAND(vvvvRegister)</div></div><div>…</div><div>}</div><div><br></div><div>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.</div>
<div>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.</div><div>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.</div>
<div>What we really want is to reverse the direction of TIED_TO for mask and mask_wb.</div><div><br></div><div>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.</div>
<div>In X86MCCodeEmitter.cpp, we increase CurOp twice if we have 2 tied-to operands. But it is kind of ugly.</div><div><div><br></div><div>+ // FIXME: Handle the special case for GATHER:</div><div>+ // For GATHER with 5 operands (dst, mask_wb, src1, mem, mask), src1 is tied</div>
<div>+ // to dst and mask is tied to mask_wb. The operandMapping is "0 to 0,</div><div>+ // 1 to 1, 2 to 0, 3 to 3, 4 to 1". The 2nd physical operand is mask_wb, and</div><div>+ // it is before mem, so we need to explicitly handle vvvvRegister first.</div>
<div>+ if (HasVEX_4VOp3Prefix && tiedOperandsCount >= 2)</div><div>+ HANDLE_OPERAND(vvvvRegister)</div><div>+</div></div><div><br></div><div>Thanks,</div><div>Manman</div><br><div><div>On Jul 9, 2012, at 11:47 PM, Craig Topper wrote:</div>
<br><blockquote type="cite">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.<br><br>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.<br>
<br><div class="gmail_quote">On Mon, Jul 9, 2012 at 11:24 PM, Manman Ren <span dir="ltr"><<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
Yes, there is an easy way to fix this.<br>
MRMSrcMem assumes register, memory, vvvv register if VEX_4VOp3 is true and assumes register, vvvv register, memory if VEX_4V is true.<br>
<br>
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:<br>
Desc->getOperandConstraint(1, MCOI::TIED_TO) != -1 (hard-coded index 1)<br>
I will fix those to handle this instruction.<br>
<br>
Thanks,<br>
Manman<br>
<div><div><br>
On Jul 9, 2012, at 10:07 PM, Evan Cheng wrote:<br>
<br>
><br>
><br>
> On Jul 9, 2012, at 4:15 PM, Manman Ren <<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>> wrote:<br>
><br>
>><br>
>> I need to implement an instruction which has 2 read-write registers, so I added<br>
>> let Constraints = "$src1 = $dst, $mask = $mask_wb" in {<br>
>> ...<br>
>> def rm : AVX28I<opc, MRMSrcMem, (outs VR128:$dst, VR128:$mask_wb),<br>
>> (ins VR128:$src1, v128mem:$src2, VR128:$mask),<br>
>> ...<br>
>> }<br>
>> There is a problem since MRMSrcMem assumes the 2nd physical operand is a memory operand.<br>
>> See the section about MRMSrcMem in RecognizableInstr::emitInstructionSpecifier.<br>
><br>
> Can this be fixed?<br>
><br>
> Evan<br>
><br>
>> And the above gives us $dst, $mask_wb, $src1, $mem, $mask, and $mask_wb is the second physical operand.<br>
>><br>
>> I thought about using "$mask_wb = $mask", but it breaks the assumption of TIED_TO LhsIdx > RhsIdx.<br>
>> Is adding another addressing mode a good idea?<br>
>><br>
>> Any pointer is appreciated.<br>
>> Thanks,<br>
>> Manman<br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank">http://llvm.cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>~Craig<br>
</blockquote></div><br></div></blockquote></div><br></div></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br>~Craig<br>
<span><gather.patch></span></blockquote></div><br></div></body></html>