[llvm-commits] [llvm] r139485 - in /llvm/trunk: lib/Target/X86/X86InstrArithmetic.td test/MC/Disassembler/X86/simple-tests.txt utils/TableGen/X86RecognizableInstr.cpp

[Bruno Cardoso Lopes]

On Mon, Sep 12, 2011 at 1:38 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Mon, Sep 12, 2011 at 1:14 AM, Bruno Cardoso Lopes
> <bruno.cardoso at gmail.com> wrote:
>> Hi Craig,
>>
>> On Sun, Sep 11, 2011 at 2:41 PM, Craig Topper <craig.topper at gmail.com> wrote:
>>> Author: ctopper
>>> Date: Sun Sep 11 16:41:45 2011
>>> New Revision: 139485
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=139485&view=rev
>>> Log:
>>> Fix disassembling of reverse register/register forms of ADD/SUB/XOR/OR/AND/SBB/ADC/CMP/MOV.
>>>
>>> Modified:
>>>    llvm/trunk/lib/Target/X86/X86InstrArithmetic.td
>>>    llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt
>>>    llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp
>>>
>>> Modified: llvm/trunk/lib/Target/X86/X86InstrArithmetic.td
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrArithmetic.td?rev=139485&r1=139484&r2=139485&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/X86/X86InstrArithmetic.td (original)
>>> +++ llvm/trunk/lib/Target/X86/X86InstrArithmetic.td Sun Sep 11 16:41:45 2011
>>> @@ -650,6 +650,15 @@
>>>   let isCodeGenOnly = 1;
>>>  }
>>>
>>> +// BinOpRR_F_Rev - Instructions like "cmp reg, reg" (reversed encoding).
>>> +class BinOpRR_F_Rev<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo>
>>> +  : ITy<opcode, MRMSrcReg, typeinfo, (outs),
>>> +        (ins typeinfo.RegClass:$src1, typeinfo.RegClass:$src2),
>>> +        mnemonic, "{$src2, $src1|$src1, $src2}", []> {
>>> +  // The disassembler should know about this, but not the asmparser.
>>> +  let isCodeGenOnly = 1;
>>> +}
>>> +
>>>  // BinOpRM - Instructions like "add reg, reg, [mem]".
>>>  class BinOpRM<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,
>>>               dag outlist, list<dag> pattern>
>>> @@ -1017,10 +1026,10 @@
>>>       def #NAME#64rr : BinOpRR_F<BaseOpc, mnemonic, Xi64, opnode>;
>>>     } // isCommutable
>>>
>>> -    def #NAME#8rr_REV  : BinOpRR_Rev<BaseOpc2, mnemonic, Xi8>;
>>> -    def #NAME#16rr_REV : BinOpRR_Rev<BaseOpc2, mnemonic, Xi16>;
>>> -    def #NAME#32rr_REV : BinOpRR_Rev<BaseOpc2, mnemonic, Xi32>;
>>> -    def #NAME#64rr_REV : BinOpRR_Rev<BaseOpc2, mnemonic, Xi64>;
>>> +    def #NAME#8rr_REV  : BinOpRR_F_Rev<BaseOpc2, mnemonic, Xi8>;
>>> +    def #NAME#16rr_REV : BinOpRR_F_Rev<BaseOpc2, mnemonic, Xi16>;
>>> +    def #NAME#32rr_REV : BinOpRR_F_Rev<BaseOpc2, mnemonic, Xi32>;
>>> +    def #NAME#64rr_REV : BinOpRR_F_Rev<BaseOpc2, mnemonic, Xi64>;
>>>
>>>     def #NAME#8rm   : BinOpRM_F<BaseOpc2, mnemonic, Xi8 , opnode>;
>>>     def #NAME#16rm  : BinOpRM_F<BaseOpc2, mnemonic, Xi16, opnode>;
>>>
>>> Modified: llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt?rev=139485&r1=139484&r2=139485&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt (original)
>>> +++ llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt Sun Sep 11 16:41:45 2011
>>> @@ -161,3 +161,15 @@
>>>
>>>  # CHECK: pause
>>>  0xf3 0x90
>>> +
>>> +# CHECK: addl %eax, %edi
>>> +0x01 0xc7
>>> +
>>> +# CHECK: addl %edi, %eax
>>> +0x03 0xc7
>>> +
>>> +# CHECK: movl %eax, %edi
>>> +0x89 0xc7
>>> +
>>> +# CHECK: movl %edi, %eax
>>> +0x8b 0xc7
>>>
>>> Modified: llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp?rev=139485&r1=139484&r2=139485&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp (original)
>>> +++ llvm/trunk/utils/TableGen/X86RecognizableInstr.cpp Sun Sep 11 16:41:45 2011
>>> @@ -345,7 +345,7 @@
>>>     return FILTER_STRONG;
>>>
>>>   if (Form == X86Local::Pseudo ||
>>> -      IsCodeGenOnly)
>>> +      (IsCodeGenOnly && Name.find("_REV") == Name.npos))
>>>     return FILTER_STRONG;
>>
>> I know that the "_REV" patterns were already there before you hack
>> into this code. We would like eventually to remove all logic dependent
>> on name suffixes or prefixes ("_REV", "_Int", "Int_" ... ), and create
>> InstAliases, patterns or even add more flags like TB, XS, and so on...
>> Since you're investigating and fixing disassembler stuff, do you have
>> ideas on how to achieve the same results without duplicating
>> instructions and appending the _REV suffix? Would that be possible?
>
> It's worth noting that these aren't really duplicates; unlike the Int_
> and _Int cases, from the perspective of x86 encoding, the _REV
> instructions are distinct instructions that happen to do the same
> thing as some other instruction.  Trying to merge them with the
> functionally equivalent instructions would require a significant
> rearchitecture which I doubt is worthwhile.

I see. Thanks for the explanation Eli!


-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc