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

[Eli Friedman]

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.

-Eli