[llvm-commits] [llvm] r139028 - in /llvm/trunk: lib/Target/X86/Disassembler/X86Disassembler.cpp lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp test/MC/Disassembler/X86/simple-tests.txt

Evan Cheng evan.cheng at apple.com
Mon Sep 5 14:52:31 PDT 2011



On Sep 4, 2011, at 1:09 PM, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Fri, Sep 2, 2011 at 1:01 PM, Kevin Enderby <enderby at apple.com> wrote:
>> Author: enderby
>> Date: Fri Sep  2 15:01:23 2011
>> New Revision: 139028
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=139028&view=rev
>> Log:
>> Change X86 disassembly to print immediates values as signed by default.  Special
>> case those instructions that the immediate is not sign-extend.  radr://8795217
>> 
>> Modified:
>>    llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp
>>    llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
>>    llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt
>> 
>> Modified: llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp?rev=139028&r1=139027&r2=139028&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp Fri Sep  2 15:01:23 2011
>> @@ -28,6 +28,8 @@
>> 
>>  #define GET_REGINFO_ENUM
>>  #include "X86GenRegisterInfo.inc"
>> +#define GET_INSTRINFO_ENUM
>> +#include "X86GenInstrInfo.inc"
>>  #include "X86GenEDInfo.inc"
>> 
>>  using namespace llvm;
>> @@ -184,6 +186,38 @@
>>       break;
>>     }
>>   }
>> +  // By default sign-extend all X86 immediates based on their encoding.
>> +  else if (type == TYPE_IMM8 || type == TYPE_IMM16 || type == TYPE_IMM32 ||
>> +           type == TYPE_IMM64) {
>> +    uint32_t Opcode = mcInst.getOpcode();
>> +    switch (operand.encoding) {
>> +    default:
>> +      break;
>> +    case ENCODING_IB:
>> +      // Special case those X86 instructions that use the imm8 as a set of
>> +      // bits, bit count, etc. and are not sign-extend.
>> +      if (Opcode != X86::BLENDPSrri && Opcode != X86::BLENDPDrri &&
>> +         Opcode != X86::PBLENDWrri && Opcode != X86::MPSADBWrri &&
>> +         Opcode != X86::DPPSrri && Opcode != X86::DPPDrri &&
>> +         Opcode != X86::INSERTPSrr && Opcode != X86::VBLENDPSYrri &&
>> +         Opcode != X86::VBLENDPSYrmi && Opcode != X86::VBLENDPDYrri &&
>> +         Opcode != X86::VBLENDPDYrmi && Opcode != X86::VPBLENDWrri &&
>> +         Opcode != X86::VMPSADBWrri && Opcode != X86::VDPPSYrri &&
>> +         Opcode != X86::VDPPSYrmi && Opcode != X86::VDPPDrri &&
>> +         Opcode != X86::VINSERTPSrr)
> 
> This can't possibly be the complete list of instructions with a
> non-extended immediate.  PCMPESTRI and friends, SHUFPS and friends,
> all shift instructions, and IN/OUT (plus possibly others I've missed)
> all take an imm8 which is clearly not signed.

It's also the wrong way to fix the bug. Kevin, can you add a new type to distinguish between extending and non-extending immediates?

Evan

> 
> -Eli
> 
>> +       type = TYPE_MOFFS8;
>> +      break;
>> +    case ENCODING_IW:
>> +      type = TYPE_MOFFS16;
>> +      break;
>> +    case ENCODING_ID:
>> +      type = TYPE_MOFFS32;
>> +      break;
>> +    case ENCODING_IO:
>> +      type = TYPE_MOFFS64;
>> +      break;
>> +    }
>> +  }
>> 
>>   switch (type) {
>>   case TYPE_MOFFS8:
>> 
>> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp?rev=139028&r1=139027&r2=139028&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp Fri Sep  2 15:01:23 2011
>> @@ -90,7 +90,8 @@
>>   if (Op.isReg()) {
>>     O << '%' << getRegisterName(Op.getReg());
>>   } else if (Op.isImm()) {
>> -    O << '$' << Op.getImm();
>> +    // Print X86 immediates as signed values.
>> +    O << '$' << (int64_t)Op.getImm();
>> 
>>     if (CommentStream && (Op.getImm() > 255 || Op.getImm() < -256))
>>       *CommentStream << format("imm = 0x%llX\n", (long long)Op.getImm());
>> 
>> 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=139028&r1=139027&r2=139028&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt (original)
>> +++ llvm/trunk/test/MC/Disassembler/X86/simple-tests.txt Fri Sep  2 15:01:23 2011
>> @@ -102,3 +102,59 @@
>> 
>>  # CHECK: vmovapd %xmm0, %xmm2
>>  0xc5 0xf9 0x28 0xd0
>> +
>> +# Check X86 immediates print as signed values by default.  radr://8795217
>> +# CHECK: andq $-16, %rsp
>> +0x48 0x83 0xe4 0xf0
>> +
>> +# Check these special case instructions that the immediate is not sign-extend.
>> +# CHECK: blendps $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x0c 0xca 0x81
>> +
>> +# CHECK: blendpd $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x0d 0xca 0x81
>> +
>> +# CHECK: pblendw $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x0e 0xca 0x81
>> +
>> +# CHECK: mpsadbw $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x42 0xca 0x81
>> +
>> +# CHECK: dpps $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x40 0xca 0x81
>> +
>> +# CHECK: dppd $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x41 0xca 0x81
>> +
>> +# CHECK: insertps $129, %xmm2, %xmm1
>> +0x66 0x0f 0x3a 0x21 0xca 0x81
>> +
>> +# CHECK: vblendps $129, %ymm2, %ymm5, %ymm1
>> +0xc4 0xe3 0x55 0x0c 0xca 0x81
>> +
>> +# CHECK: vblendps $129, (%rax), %ymm5, %ymm1
>> +0xc4 0xe3 0x55 0x0c 0x08 0x81
>> +
>> +# CHECK: vblendpd $129, %ymm2, %ymm5, %ymm1
>> +0xc4 0xe3 0x55 0x0d 0xca 0x81
>> +
>> +# CHECK: vblendpd $129, (%rax), %ymm5, %ymm1
>> +0xc4 0xe3 0x55 0x0d 0x08 0x81
>> +
>> +# CHECK: vpblendw $129, %xmm2, %xmm5, %xmm1
>> +0xc4 0xe3 0x51 0x0e 0xca 0x81
>> +
>> +# CHECK: vmpsadbw $129, %xmm2, %xmm5, %xmm1
>> +0xc4 0xe3 0x51 0x42 0xca 0x81
>> +
>> +# CHECK: vdpps $129, %ymm2, %ymm5, %ymm1
>> +0xc4 0xe3 0x55 0x40 0xca 0x81
>> +
>> +# CHECK: vdpps $129, (%rax), %ymm5, %ymm1
>> +0xc4 0xe3 0x55 0x40 0x08 0x81
>> +
>> +# CHECK: vdppd $129, %xmm2, %xmm5, %xmm1
>> +0xc4 0xe3 0x51 0x41 0xca 0x81
>> +
>> +# CHECK: vinsertps $129, %xmm3, %xmm2, %xmm1
>> +0xc4 0xe3 0x69 0x21 0xcb 0x81
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list