[llvm-commits] XOP encoding patch

[Jan Sjodin]

Here is an updated patch with XOP8 and XOP9. It looks cleaner. Let me know what you think.

- Jan



----- Original Message -----
> From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
> To: Eli Friedman <eli.friedman at gmail.com>
> Cc: Jan Sjodin <jan_sjodin at yahoo.com>; "llvm-commits at cs.uiuc.edu" <llvm-commits at cs.uiuc.edu>
> Sent: Thursday, December 8, 2011 8:50 PM
> Subject: Re: [llvm-commits] XOP encoding patch
> 
> Hi,
> 
> On Thu, Dec 8, 2011 at 7:06 PM, Eli Friedman <eli.friedman at gmail.com> 
> wrote:
>>  On Thu, Dec 8, 2011 at 12:49 PM, Jan Sjodin <jan_sjodin at yahoo.com> 
> wrote:
>>>  This patch handles the encoding of XOP instructions. I included one
>>>  instruction for each kind including a test.
>> 
>>  +multiclass xop4opimm<bits<8> opc, string OpcodeStr> {
>>  +  def ri : IXOPi8<opc, MRMSrcReg, (outs VR128:$dst),
>>  +           (ins VR128:$src1, VR128:$src2, i32i8imm:$src3),
>> 
>>  As discussed on IRC, you want i8imm here, not i32i8imm.
>> 
>>  +      // If there is an additional 5th operand it must be an immediate, 
> which
>>  +      // is encoded in bits[3:0]
>>  +      if(CurOp != NumOps) {
>>  +       const MCOperand &MIMM = MI.getOperand(CurOp++);
>>  +       if(MIMM.isImm()) {
>>  +         unsigned Val = MIMM.getImm();
>>  +         assert(Val < 16 && "Immediate operand value out 
> of range");
>>  +         RegNum |= Val;
>>  +       }
>>  +      }
>> 
>>  No tabs, please.
>> 
>>  This patch doesn't include disassembler support; are you planning to 
> add that?
>> 
>>  Someone more familiar with the x86 instruction patterns should
>>  probably review this in addition.
> 
> Regarding the MC code emitter, my suggestion here is to factor out
> common code from EmitVEXOpcodePrefix, and have a new
> EmitXOPOpcodePrefix method which reuses the common part. IMO adding
> several tricky conditions to get around the differences between them
> isn't the way to go (and compromisses the code readability). That
> means whenever one of the VEX prefixes has a slightly different
> meaning in XOP, there should be a new TSFlags for it.
> 
> -- 
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0058_xop_encoding.patch
Type: application/octet-stream
Size: 20237 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111209/7b0838ac/attachment.obj>