[llvm-commits] [PATCH]Add support for fstmx/fldmx instructions on the ARM backend

Jim Grosbach grosbach at apple.com
Fri Jun 22 12:42:33 PDT 2012


Hi Silviu

OK.  More detailed comments on the specifics below.


On May 24, 2012, at 5:21 AM, Silviu Baranga <silbar01 at arm.com> wrote:

> Hi Jim,
>  
> Thanks for the review.
>  
> These instructions also support the “DB” suffix, and having
> either IA/DB is a required part of the syntax.
>  
> The tests from fstmx-arm.txt were supposed to test that
> some invalid bit patterns are not decoded as vstm/vldm
> instructions (which is  the current behaviour).
> Those bit patterns are invalid because they have an odd
> imm8 field (so they are not vstm/vldm instructions) and
> the value of the 22nd bit is 1 (so they are not fstmx/fldmx
> instructions).
>  
> I’ve removed the fstmx-arm.txt test, and added instead
> the invalid-fstmx.txt test file (the new patch is attached).

> Index: test/MC/ARM/simple-fp-encoding.s
> ===================================================================
> --- test/MC/ARM/simple-fp-encoding.s	(revision 156615)
> +++ test/MC/ARM/simple-fp-encoding.s	(working copy)
> @@ -352,3 +352,12 @@
>  
>  @ CHECK: vmov.i32	d4, #0x0        @ encoding: [0x10,0x40,0x80,0xf2]
>  @ CHECK: vmov.i32	d4, #0x42000000 @ encoding: [0x12,0x46,0x84,0xf2]
> +
> +        fldmiaxeq r0, {d4,d5}
> +        fstmiaxeq r4, {d8,d9}
> +        fldmdbxne r5!, {d4,d5,d6}
> +        fstmdbxne r7!, {d2-d4}
> +@ CHECK: fldmiaxeq r0, {d4-d5}          @ encoding: [0x05,0x4b,0x90,0x0c]
> +@ CHECK: fstmiaxeq r4, {d8-d9}          @ encoding: [0x05,0x8b,0x84,0x0c]
> +@ CHECK: fldmdbxne r5!, {d4-d6}         @ encoding: [0x07,0x4b,0x35,0x1d]
> +@ CHECK: fstmdbxne r7!, {d2-d4}         @ encoding: [0x07,0x2b,0x27,0x1d]
> Index: test/MC/Disassembler/ARM/invalid-fstmx.txt
> ===================================================================
> --- test/MC/Disassembler/ARM/invalid-fstmx.txt	(revision 0)
> +++ test/MC/Disassembler/ARM/invalid-fstmx.txt	(revision 0)
> @@ -0,0 +1,4 @@
> +# RUN: llvm-mc --disassemble %s -triple=armv7-linux-gnueabi |& FileCheck %s
> +
> +0x0d 0x0b 0xc7 0x0c
> +# CHECK: invalid instruction encoding
> Index: test/MC/Disassembler/ARM/fp-encoding.txt
> ===================================================================
> --- test/MC/Disassembler/ARM/fp-encoding.txt	(revision 156615)
> +++ test/MC/Disassembler/ARM/fp-encoding.txt	(working copy)
> @@ -238,3 +238,16 @@
>  # CHECK: vcvtr.s32.f32  s0, s1
>  # CHECK: vcvtr.u32.f64  s0, d0
>  # CHECK: vcvtr.u32.f32  s0, s1
> +
> +0x0f 0x3b 0xb7 0x0c
> +0x0d 0x4b 0x96 0x0c
> +0x09 0x1b 0x38 0xed
> +0x07 0x2b 0x83 0xec
> +0x05 0x5b 0xa3 0x0c
> +0x0f 0x3b 0x20 0x1d
> +# CHECK: fldmiaxeq r7!, {d3-d9}
> +# CHECK: fldmiaxeq r6, {d4-d9}
> +# CHECK: fldmdbx   r8!, {d1-d4}
> +# CHECK: fstmiax   r3, {d2-d4}
> +# CHECK: fstmiaxeq r3!, {d5-d6}
> +# CHECK: fstmdbxne r0!, {d3-d9}
> Index: utils/TableGen/EDEmitter.cpp
> ===================================================================
> --- utils/TableGen/EDEmitter.cpp	(revision 156615)
> +++ utils/TableGen/EDEmitter.cpp	(working copy)
> @@ -700,6 +700,7 @@
>    MISC("addr_offset_none", "kOperandTypeARMAddrMode7");           // R
>    MISC("reglist", "kOperandTypeARMRegisterList");                 // I, R, ...
>    MISC("dpr_reglist", "kOperandTypeARMDPRRegisterList");          // I, R, ...
> +  MISC("dpr_reglist_fmx", "kOperandTypeARMDPRRegisterList");      // I, R, ...
>    MISC("spr_reglist", "kOperandTypeARMSPRRegisterList");          // I, R, ...
>    MISC("it_mask", "kOperandTypeThumbITMask");                     // I
>    MISC("t2addrmode_reg", "kOperandTypeThumb2AddrModeReg");        // R
> Index: lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> ===================================================================
> --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp	(revision 156615)
> +++ lib/Target/ARM/AsmParser/ARMAsmParser.cpp	(working copy)
> @@ -849,6 +849,26 @@
>    bool isReg() const { return Kind == k_Register; }
>    bool isRegList() const { return Kind == k_RegisterList; }
>    bool isDPRRegList() const { return Kind == k_DPRRegisterList; }
> +  bool isDPRRegListFMX() const {
> +    // We need to check that we don't use a D reg with an index greater
> +    // then 15.
> +    if (Kind != k_DPRRegisterList)
> +      return false;
> +    const SmallVectorImpl<unsigned> &list = getRegList();
> +    for (SmallVectorImpl<unsigned>::const_iterator I = list.begin(), 
> +                                              E = list.end();
> +                                              I != E;
> +                                              ++I){
> +      // We only allow D registers smaller then 16.
> +      if (!(ARMMCRegisterClasses[ARM::DPRRegClassID].contains(*I))) 
> +        return false;
> +      if (getARMRegisterNumbering(*I) > 15) return false; 

We have the DPR_VFP2 register class for the low 16 DPR registers. You can just update the .contains() check instead.

Relatedly, the name of the operand type shouldn't reflect the instructions, but the nature of the registers. Since this is a list of DPR_VFP2 registers, "DPR_VFP2RegList" seems to make a certain amount of sense.

> +    }
> +    // No registers were transfered.

"transferred". That said, I'm not sure what this comment means. No transference is occurring here.

> +    if (list.size() == 0) return false;

This can be an assert() at the top of the function if it's here at all. An empty register list should result in an error in parsing and never be created as an asmoperand.

> +    
> +    return true;
> +  }
>    bool isSPRRegList() const { return Kind == k_SPRRegisterList; }
>    bool isToken() const { return Kind == k_Token; }
>    bool isMemBarrierOpt() const { return Kind == k_MemBarrierOpt; }
> @@ -1472,6 +1492,16 @@
>      addRegListOperands(Inst, N);
>    }
>  
> +  void addDPRRegListFMXOperands(MCInst &Inst, unsigned N) const {
> +    const SmallVectorImpl<unsigned> &RegList = getRegList();
> +    assert(RegList.size() > 0 && "Register list size must be greater then 0.");
> +
> +    // Add the operand indicating the first register.
> +    Inst.addOperand(MCOperand::CreateReg(RegList[0]));
> +    // Add the immediate operand indicating the number of registers used.
> +    Inst.addOperand(MCOperand::CreateImm(RegList.size()));
> +  }
> +

Why is this represented differently than the existing register lists for vldm/vstm? That adds a lot of unnecessary complexity to the patch. Just use the existing infrastructure.

>    void addSPRRegListOperands(MCInst &Inst, unsigned N) const {
>      addRegListOperands(Inst, N);
>    }
> Index: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
> ===================================================================
> --- lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp	(revision 156615)
> +++ lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp	(working copy)
> @@ -603,6 +603,27 @@
>    O << ", asr #" << Imm;
>  }
>  
> +void ARMInstPrinter::printRegisterFMXList(const MCInst *MI, unsigned OpNum,
> +                                       raw_ostream &O) {
> +  unsigned BaseReg = MI->getOperand(OpNum).getReg();
> +  unsigned NumRegs = MI->getOperand(OpNum + 1).getImm();
> +
> +  O << "{";
> +  // We will write this with the - format, since the upper registers may not
> +  // exist.
> +  switch(NumRegs){
> +    case 0:
> +      break;
> +    case 1:
> +      O << getRegisterName(BaseReg);
> +      break;
> +    default:
> +      O << getRegisterName(BaseReg);
> +      O << "-d" << getARMRegisterNumbering(BaseReg) + NumRegs - 1;
> +  }
> +  O << "}";
> +}
> +

This can go away and the operand can just reference the existing printRegisterList().

>  void ARMInstPrinter::printRegisterList(const MCInst *MI, unsigned OpNum,
>                                         raw_ostream &O) {
>    O << "{";
> Index: lib/Target/ARM/InstPrinter/ARMInstPrinter.h
> ===================================================================
> --- lib/Target/ARM/InstPrinter/ARMInstPrinter.h	(revision 156615)
> +++ lib/Target/ARM/InstPrinter/ARMInstPrinter.h	(working copy)
> @@ -114,6 +114,8 @@
>                                        raw_ostream &O);
>    void printSBitModifierOperand(const MCInst *MI, unsigned OpNum,
>                                  raw_ostream &O);
> +  void printRegisterFMXList(const MCInst *MI, unsigned OpNum,
> +                                       raw_ostream &O);
>    void printRegisterList(const MCInst *MI, unsigned OpNum, raw_ostream &O);
>    void printNoHashImmediate(const MCInst *MI, unsigned OpNum, raw_ostream &O);
>    void printPImmediate(const MCInst *MI, unsigned OpNum, raw_ostream &O);
> Index: lib/Target/ARM/ARMInstrVFP.td
> ===================================================================
> --- lib/Target/ARM/ARMInstrVFP.td	(revision 156615)
> +++ lib/Target/ARM/ARMInstrVFP.td	(working copy)
> @@ -118,7 +118,7 @@
>                           InstrItinClass itin, InstrItinClass itin_upd> {
>    // Double Precision
>    def DIA :
> -    AXDI4<(outs), (ins GPR:$Rn, pred:$p, dpr_reglist:$regs, variable_ops),
> +    AXDI4_D<(outs), (ins GPR:$Rn, pred:$p, dpr_reglist:$regs, variable_ops),
>            IndexModeNone, itin,
>            !strconcat(asm, "ia${p}\t$Rn, $regs"), "", []> {
>      let Inst{24-23} = 0b01;       // Increment After
> @@ -126,7 +126,7 @@
>      let Inst{20}    = L_bit;
>    }
>    def DIA_UPD :
> -    AXDI4<(outs GPR:$wb), (ins GPR:$Rn, pred:$p, dpr_reglist:$regs,
> +    AXDI4_D<(outs GPR:$wb), (ins GPR:$Rn, pred:$p, dpr_reglist:$regs,
>                                 variable_ops),
>            IndexModeUpd, itin_upd,
>            !strconcat(asm, "ia${p}\t$Rn!, $regs"), "$Rn = $wb", []> {
> @@ -135,7 +135,7 @@
>      let Inst{20}    = L_bit;
>    }
>    def DDB_UPD :
> -    AXDI4<(outs GPR:$wb), (ins GPR:$Rn, pred:$p, dpr_reglist:$regs,
> +    AXDI4_D<(outs GPR:$wb), (ins GPR:$Rn, pred:$p, dpr_reglist:$regs,
>                                 variable_ops),
>            IndexModeUpd, itin_upd,
>            !strconcat(asm, "db${p}\t$Rn!, $regs"), "$Rn = $wb", []> {
> @@ -216,7 +216,41 @@
>                           (VLDMDIA_UPD SP, pred:$p, dpr_reglist:$r)>;
>  
>  // FLDMX, FSTMX - mixing S/D registers for pre-armv6 cores
> +// These instructions are deprecated, so we don't want them to get selected.
>  

This comment confuses me. Are these instructions able to reference the S registers as well as the D registers? Nothing in this patch handles that, as far as I can tell. Everything talks about D registers.


> +multiclass vfp_fldst_mult<string asm, bit L_bit,
> +                         InstrItinClass itin, InstrItinClass itin_upd> {
> +  // Double Precision
> +  def DIA :
> +    AXDI4_F<(outs), (ins GPR:$Rn, pred:$p, dpr_reglist_fmx:$regs, variable_ops),
> +          IndexModeNone, itin,
> +          !strconcat(asm, "iax${p}\t$Rn, $regs"), "", []> {
> +    let Inst{24-23} = 0b01;       // Increment After
> +    let Inst{21}    = 0;          // No writeback
> +    let Inst{20}    = L_bit;
> +  }
> +  def DIA_UPD :
> +    AXDI4_F<(outs GPR:$wb), (ins GPR:$Rn, pred:$p, dpr_reglist_fmx:$regs,
> +                               variable_ops),
> +          IndexModeUpd, itin_upd,
> +          !strconcat(asm, "iax${p}\t$Rn!, $regs"), "$Rn = $wb", []> {
> +    let Inst{24-23} = 0b01;       // Increment After
> +    let Inst{21}    = 1;          // Writeback
> +    let Inst{20}    = L_bit;
> +  }
> +  def DDB_UPD :
> +    AXDI4_F<(outs GPR:$wb), (ins GPR:$Rn, pred:$p, dpr_reglist_fmx:$regs,
> +                               variable_ops),
> +          IndexModeUpd, itin_upd,
> +          !strconcat(asm, "dbx${p}\t$Rn!, $regs"), "$Rn = $wb", []> {
> +    let Inst{24-23} = 0b10;       // Decrement Before
> +    let Inst{21}    = 1;          // Writeback
> +    let Inst{20}    = L_bit;
> +  }
> +}
> +
> +defm FLDM : vfp_fldst_mult<"fldm", 1, NoItinerary, NoItinerary>;
> +defm FSTM : vfp_fldst_mult<"fstm", 0, NoItinerary, NoItinerary>;
>  //===----------------------------------------------------------------------===//
>  // FP Binary Operations.
>  //
> Index: lib/Target/ARM/ARMInstrInfo.td
> ===================================================================
> --- lib/Target/ARM/ARMInstrInfo.td	(revision 156615)
> +++ lib/Target/ARM/ARMInstrInfo.td	(working copy)
> @@ -386,6 +386,14 @@
>    let DecoderMethod = "DecodeDPRRegListOperand";
>  }
>  
> +def DPRRegListFMXAsmOperand : AsmOperandClass {let Name = "DPRRegListFMX"; }
> +def dpr_reglist_fmx : Operand<i32> {
> +  let EncoderMethod = "getRegisterFMXListOpValue";
> +  let ParserMatchClass = DPRRegListFMXAsmOperand;
> +  let PrintMethod = "printRegisterFMXList";
> +  let DecoderMethod = "DecodeDPRRegFMXListOperand";

Other than the AsmOperandClass, these can all refer to the same functions as for the normal DPRRegList.

> +}
> +
>  def SPRRegListAsmOperand : AsmOperandClass { let Name = "SPRRegList"; }
>  def spr_reglist : Operand<i32> {
>    let EncoderMethod = "getRegisterListOpValue";
> Index: lib/Target/ARM/ARMCodeEmitter.cpp
> ===================================================================
> --- lib/Target/ARM/ARMCodeEmitter.cpp	(revision 156615)
> +++ lib/Target/ARM/ARMCodeEmitter.cpp	(working copy)
> @@ -320,6 +320,8 @@
>      unsigned getNEONVcvtImm32OpValue(const MachineInstr &MI, unsigned Op)
>        const { return 0; }
>  
> +    unsigned getRegisterFMXListOpValue(const MachineInstr &MI, unsigned Op)
> +      const { return 0; }
>      unsigned getRegisterListOpValue(const MachineInstr &MI, unsigned Op)
>        const { return 0; }
>  
> Index: lib/Target/ARM/ARMInstrFormats.td
> ===================================================================
> --- lib/Target/ARM/ARMInstrFormats.td	(revision 156615)
> +++ lib/Target/ARM/ARMInstrFormats.td	(working copy)
> @@ -1426,16 +1426,32 @@
>  
>    // Encode instruction operands.
>    let Inst{19-16} = Rn;
> -  let Inst{22}    = regs{12};
>    let Inst{15-12} = regs{11-8};
> -  let Inst{7-0}   = regs{7-0};
> -
> +  let Inst{7-1} = regs{7-1};

Where do the bits being removed here get set for the old instructions from the vfp_ldst_mult multiclass?

>    // TODO: Mark the instructions with the appropriate subtarget info.
>    let Inst{27-25} = 0b110;
>    let Inst{11-9}  = 0b101;
>    let Inst{8}     = 1;          // Double precision
> +  
> +  let regs{0} = 0;

What is this supposed to be doing? We shouldn't be changing the value of regs here. Operand values should be considered to be const.

>  }
>  
> +class AXDI4_D<dag oops, dag iops, IndexMode im, InstrItinClass itin,
> +              string asm, string cstr, list<dag> pattern>
> +  : AXDI4<oops, iops, im, itin, asm, cstr, pattern> {
> +  let Inst{22} = regs{12};
> +  let Inst{12} = regs{8};

Inst{12} is still set in the base class. Why set it again here?

> +  let Inst{0} = 0;
> +}
> +
> +class AXDI4_F<dag oops, dag iops, IndexMode im, InstrItinClass itin,
> +              string asm, string cstr, list<dag> pattern>
> +  : AXDI4<oops, iops, im, itin, asm, cstr, pattern> {
> +  let Inst{22} = 0;
> +  let Inst{0} = 1;
> +  let regs{12} = 0;
> +}
> +
>  class AXSI4<dag oops, dag iops, IndexMode im, InstrItinClass itin,
>              string asm, string cstr, list<dag> pattern>
>    : VFPXI<oops, iops, AddrMode4, 4, im,
> Index: lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
> ===================================================================
> --- lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp	(revision 156615)
> +++ lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp	(working copy)
> @@ -283,7 +283,9 @@
>  
>    unsigned getBitfieldInvertedMaskOpValue(const MCInst &MI, unsigned Op,
>                                        SmallVectorImpl<MCFixup> &Fixups) const;
> -
> +  
> +  unsigned getRegisterFMXListOpValue(const MCInst &MI, unsigned Op,
> +                                     SmallVectorImpl<MCFixup> &Fixups) const;
>    unsigned getRegisterListOpValue(const MCInst &MI, unsigned Op,
>                                    SmallVectorImpl<MCFixup> &Fixups) const;
>    unsigned getAddrMode6AddressOpValue(const MCInst &MI, unsigned Op,
> @@ -1335,6 +1337,21 @@
>    return lsb | (msb << 5);
>  }
>  
> +unsigned ARMMCCodeEmitter:: getRegisterFMXListOpValue(const MCInst &MI, 
> +                      unsigned Op, SmallVectorImpl<MCFixup> &Fixups) const {
> +
> +  unsigned Reg = MI.getOperand(Op).getReg();
> +  unsigned NumRegs = MI.getOperand(Op + 1).getImm();
> +
> +  unsigned RegNo = getARMRegisterNumbering(Reg);
> +  unsigned Binary = 0;
> +
> +  Binary |= (RegNo & 0x1f) << 8;
> +  Binary |= NumRegs * 2;
> +
> +  return Binary;
> +}
> +

This can just use the encoder already present for DPR lists. No need for a new one.

>  unsigned ARMMCCodeEmitter::
>  getRegisterListOpValue(const MCInst &MI, unsigned Op,
>                         SmallVectorImpl<MCFixup> &Fixups) const {
> Index: lib/Target/ARM/Disassembler/ARMDisassembler.cpp
> ===================================================================
> --- lib/Target/ARM/Disassembler/ARMDisassembler.cpp	(revision 156615)
> +++ lib/Target/ARM/Disassembler/ARMDisassembler.cpp	(working copy)
> @@ -197,7 +197,10 @@
>                                 uint64_t Address, const void *Decoder);
>  static DecodeStatus DecodeDPRRegListOperand(MCInst &Inst, unsigned Val,
>                                 uint64_t Address, const void *Decoder);
> +static DecodeStatus DecodeDPRRegFMXListOperand(MCInst &Inst, unsigned Val,
> +                               uint64_t Address, const void *Decoder);
>  
> +
>  static DecodeStatus DecodeBitfieldMaskOperand(MCInst &Inst, unsigned Insn,
>                                 uint64_t Address, const void *Decoder);
>  static DecodeStatus DecodeCopMemInstruction(MCInst &Inst, unsigned Insn,
> @@ -1237,18 +1240,38 @@
>    return S;
>  }
>  
> +static DecodeStatus DecodeDPRRegFMXListOperand(MCInst &Inst, unsigned Val,
> +                               uint64_t Address, const void *Decoder){
> +  DecodeStatus S = MCDisassembler::Success;
> +
> +  unsigned Vd = fieldFromInstruction32(Val, 8, 4);
> +  int regs = fieldFromInstruction32(Val, 1, 7);
> +
> +  if (regs == 0 || Vd + regs > 16)
> +    S = MCDisassembler::SoftFail;
> +
> +  if (!Check(S, DecodeDPRRegisterClass(Inst, Vd, Address, Decoder)))
> +      return MCDisassembler::Fail;
> +
> +  Inst.addOperand(MCOperand::CreateImm(regs));
> +  
> +  return S;
> +}
> +

Likewise.

>  static DecodeStatus DecodeDPRRegListOperand(MCInst &Inst, unsigned Val,
>                                   uint64_t Address, const void *Decoder) {
>    DecodeStatus S = MCDisassembler::Success;
>  
>    unsigned Vd = fieldFromInstruction32(Val, 8, 5);
> -  unsigned regs = fieldFromInstruction32(Val, 0, 8);
> +  int regs = fieldFromInstruction32(Val, 1, 7);
>  
> -  regs = regs >> 1;
> +  if (regs == 0 || regs > 16 || Vd+regs > 32) 
> +    S = MCDisassembler::SoftFail;
>  
>    if (!Check(S, DecodeDPRRegisterClass(Inst, Vd, Address, Decoder)))
>        return MCDisassembler::Fail;
> -  for (unsigned i = 0; i < (regs - 1); ++i) {
> + 
> +  for (int i = 0; i < (regs - 1); ++i) {
>      if (!Check(S, DecodeDPRRegisterClass(Inst, ++Vd, Address, Decoder)))
>        return MCDisassembler::Fail;
>    }



>  
> Thanks,
> Silviu
>  
> From: Jim Grosbach [mailto:grosbach at apple.com] 
> Sent: 22 May 2012 17:32
> To: Silviu Baranga
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH]Add support for fstmx/fldmx instructions on the ARM backend
>  
> Hi Silviu,
>  
> Do these instructions only support the "IA" suffix? Is that suffix always a required part of the syntax?
>  
> Index: test/MC/Disassembler/ARM/fstmx-arm.txt
> ===================================================================
> --- test/MC/Disassembler/ARM/fstmx-arm.txt         (revision 0)
> +++ test/MC/Disassembler/ARM/fstmx-arm.txt      (revision 0)
> @@ -0,0 +1,9 @@
> +# RUN: echo "0x0d 0x0b 0xc7 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x0b 0x5b 0xd2 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x07 0x1b 0x69 0x0d" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x09 0xeb 0x37 0x0d" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x0d 0xfb 0xa4 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x0b 0x3b 0xfb 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +
> +# CHECK-NOT: vstm
> +# CHECK-NOT: vldm
>  
> Why all the separate run lines with 'echo' instead of having the bytes in the file directly, one instruction per line, like the other tests? Additionally, this should be checking what the disassembly should be, not just what it should not be. i.e., CHECK lines as well as CHECK-NOT. Relatedly, what is this testing that the entries in fl-encoding.txt do not cover?
>  
> -Jim
>  
> On May 22, 2012, at 4:05 AM, Silviu Baranga <silbar01 at arm.com> wrote:
> 
> 
> Hi,
>  
> The fstmx/fldmx ARM instructions are currently not implemented in the ARM backend.
> Although these instructions are deprecated, they should still be supported for assembly and
> disassembly purposes.
>  
> The patch adds support for these instructions and also fixes a problem with the
> vstm/vldm instructions which were colliding with the fstmx/fldmx encoding space.
>  
> Please review this patch.
>  
> Thanks,
> Silviu
> <fstmx.diff>
>  
> <fstmx.diff>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120622/9c20ee46/attachment.html>


More information about the llvm-commits mailing list