[llvm-commits] [llvm] r58626 - in /llvm/trunk/lib/Target/ARM: ARMCodeEmitter.cpp ARMInstrFormats.td ARMInstrInfo.h ARMInstrInfo.td

Jim Grosbach grosbach at apple.com
Mon Nov 3 14:10:33 PST 2008


On Nov 3, 2008, at 12:44 PM, Evan Cheng wrote:

> Ok. It's not just a naming issue. AddrMode information is used by
> other ARM routines to determine instruction properties. Currently
> AddrMode6 is only used by the code emitter to determine instruction
> encoding. That seems wrong. You should be using instruction format
> bits to determine how to encode these instructions instead. See
>

The places where the addressing mode is used in the backend are:

ARMConstantIslands::InitialFunctionScan() - This is checking the max  
PC relative displacement for constant operands for a given  
instruction. The addressing mode check is performed when a constant  
pool operand is found. Since none of these instructions have constant  
operands, this check will never be hit and it is correct to omit  
reference to it in the switch statement (just like AddrMode4 is  
omitted).

ARMRegisterInfo::eliminateFrameIndex() - Here we're looking for frame  
indexing and adjusting the instructions to get their operands relative  
to the stack pointer instead. Since all operands for these  
instructions are register direct, this does not apply.

ARMRegisterInfo::processFunctionBeforeCalleeSavedScan() - Check for  
AddrMode3 and AddrMode5 explicitly to make sure enough registers are  
available for materializing stack offsets when necessary. Since these  
instructions don't do anything like that, again, there's nothing to  
add here.

ARMInstrInfo::convertToThreeAddress() - Indexed load/store  
instructions are converted to an add/subtract with a non-indexed load/ 
store. Not applicable to the instructions we're looking at for these  
encodings.

ARMCodeEmitter::getInstrBinary() - Dispatch routine for instruction  
encoding. This is currently based exclusively on AddrMode, and calls  
out to a separate function for each one. This is the one place where  
we do need to hook in for the new instructions.

The instruction format bits are currently used to determine how to  
format and encode individual operands within a class of instructions.  
For example, my intent here is to use that format mask to handle bits  
5 and 6 for the SMLAxy, SMLAWy, SULWy, etc. instructions.

That is, the usage pattern in place is to use the addressing mode bits  
to indicate a class of instructions with common operand schemes, then  
use the format mask to break down nuances of those encodings. That's  
the pattern I've attempted to stay consistent with.

It is, of course, technically possible to just hook off from  
getAddrModeNoneInstrBinary() based on the format mask entirely. I  
chose not to do that for the reasons outlined above.

Do you have a counter-example that demonstrates why this is the wrong  
approach?


>
> // Format specifies the encoding used by the instruction.  This is
> part of the
> // ad-hoc solution used to emit machine instruction encodings by our
> machine
> // code emitter.
> class Format<bits<5> val> {
>   bits<5> Value = val;
> }
>
> def Pseudo      : Format<1>;
> def MulFrm      : Format<2>;
> def MulSMLAW    : Format<3>;
>
> Evan
>
>
> On Nov 3, 2008, at 11:41 AM, Jim Grosbach wrote:
>
>> I have no real preference for what name to use. I chose that to be
>> consistent with the other operations doing similar things (choosing
>> how to encode operands for a class of instructions). If there's an
>> alternative nomenclature that's preferable, that's fine with me.
>>
>>
>> On Nov 3, 2008, at 11:17 AM, Evan Cheng wrote:
>>
>>> Thanks. But is addrmode6 the right name? I don't see ARM manuals
>>> using
>>> that name.
>>>
>>> Evan
>>>
>>> On Nov 3, 2008, at 10:38 AM, Jim Grosbach wrote:
>>>
>>>> Author: grosbach
>>>> Date: Mon Nov  3 12:38:31 2008
>>>> New Revision: 58626
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=58626&view=rev
>>>> Log:
>>>> Add binary encoding support for multiply instructions. Some blanks
>>>> left to fill in, but the basics are there.
>>>>
>>>> Modified:
>>>> llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp
>>>> llvm/trunk/lib/Target/ARM/ARMInstrFormats.td
>>>> llvm/trunk/lib/Target/ARM/ARMInstrInfo.h
>>>> llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
>>>>
>>>> Modified: llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp?rev=58626&r1=58625&r2=58626&view=diff
>>>>
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> = 
>>>> ===================================================================
>>>> --- llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp (original)
>>>> +++ llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp Mon Nov  3  
>>>> 12:38:31
>>>> 2008
>>>> @@ -82,8 +82,8 @@
>>>>                                  const TargetInstrDesc &TID,
>>>>                                  const MachineOperand &MO);
>>>>
>>>> -    unsigned getAddrMode1SBit(const MachineInstr &MI,
>>>> -                              const TargetInstrDesc &TID) const;
>>>> +    unsigned getAddrModeSBit(const MachineInstr &MI,
>>>> +                             const TargetInstrDesc &TID) const;
>>>>
>>>>  unsigned getAddrMode1InstrBinary(const MachineInstr &MI,
>>>>                                   const TargetInstrDesc &TID,
>>>> @@ -97,6 +97,9 @@
>>>>  unsigned getAddrMode4InstrBinary(const MachineInstr &MI,
>>>>                                   const TargetInstrDesc &TID,
>>>>                                   unsigned Binary);
>>>> +    unsigned getAddrMode6InstrBinary(const MachineInstr &MI,
>>>> +                                     const TargetInstrDesc &TID,
>>>> +                                     unsigned Binary);
>>>>
>>>>  /// getInstrBinary - Return binary encoding for the specified
>>>>  /// machine instruction.
>>>> @@ -432,8 +435,8 @@
>>>> return Binary;
>>>> }
>>>>
>>>> -unsigned ARMCodeEmitter::getAddrMode1SBit(const MachineInstr &MI,
>>>> -                                          const TargetInstrDesc
>>>> &TID) const {
>>>> +unsigned ARMCodeEmitter::getAddrModeSBit(const MachineInstr &MI,
>>>> +                                         const TargetInstrDesc
>>>> &TID) const {
>>>> for (unsigned i = MI.getNumOperands(), e = TID.getNumOperands();
>>>> i != e; --i){
>>>>  const MachineOperand &MO = MI.getOperand(i-1);
>>>>  if (MO.isReg() && MO.isDef() && MO.getReg() == ARM::CPSR)
>>>> @@ -449,7 +452,7 @@
>>>> Binary |= II->getPredicate(&MI) << 28;
>>>>
>>>> // Encode S bit if MI modifies CPSR.
>>>> -  Binary |= getAddrMode1SBit(MI, TID);
>>>> +  Binary |= getAddrModeSBit(MI, TID);
>>>>
>>>> // Encode register def if there is one.
>>>> unsigned NumDefs = TID.getNumDefs();
>>>> @@ -618,6 +621,33 @@
>>>> return Binary;
>>>> }
>>>>
>>>> +unsigned ARMCodeEmitter::getAddrMode6InstrBinary(const  
>>>> MachineInstr
>>>> &MI,
>>>> +                                                 const
>>>> TargetInstrDesc &TID,
>>>> +                                                 unsigned  
>>>> Binary) {
>>>> +  // Set the conditional execution predicate
>>>> +  Binary |= II->getPredicate(&MI) << 28;
>>>> +
>>>> +  // Encode S bit if MI modifies CPSR.
>>>> +  Binary |= getAddrModeSBit(MI, TID);
>>>> +
>>>> +  // 32x32->64bit operations have two destination registers. The
>>>> number
>>>> +  // of register definitions will tell us if that's what we're
>>>> dealing with.
>>>> +  int OpIdx = 0;
>>>> +  if (TID.getNumDefs() == 2)
>>>> +    Binary |= getMachineOpValue (MI, OpIdx++) <<
>>>> ARMII::RegRdLoShift;
>>>> +
>>>> +  // Encode Rd
>>>> +  Binary |= getMachineOpValue(MI, OpIdx++) << ARMII::RegRdHiShift;
>>>> +
>>>> +  // Encode Rm
>>>> +  Binary |= getMachineOpValue(MI, OpIdx++);
>>>> +
>>>> +  // Encode Rs
>>>> +  Binary |= getMachineOpValue(MI, OpIdx++) << ARMII::RegRsShift;
>>>> +
>>>> +  return Binary;
>>>> +}
>>>> +
>>>> /// getInstrBinary - Return binary encoding for the specified
>>>> /// machine instruction.
>>>> unsigned ARMCodeEmitter::getInstrBinary(const MachineInstr &MI) {
>>>> @@ -636,6 +666,8 @@
>>>>  return getAddrMode3InstrBinary(MI, TID, Binary);
>>>> case ARMII::AddrMode4:
>>>>  return getAddrMode4InstrBinary(MI, TID, Binary);
>>>> +  case ARMII::AddrMode6:
>>>> +    return getAddrMode6InstrBinary(MI, TID, Binary);
>>>> }
>>>>
>>>> abort();
>>>>
>>>> Modified: llvm/trunk/lib/Target/ARM/ARMInstrFormats.td
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrFormats.td?rev=58626&r1=58625&r2=58626&view=diff
>>>>
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> = 
>>>> ===================================================================
>>>> --- llvm/trunk/lib/Target/ARM/ARMInstrFormats.td (original)
>>>> +++ llvm/trunk/lib/Target/ARM/ARMInstrFormats.td Mon Nov  3  
>>>> 12:38:31
>>>> 2008
>>>> @@ -659,6 +659,28 @@
>>>> let Inst{27-25} = 0b100;
>>>> }
>>>>
>>>> +// addrmode6
>>>> +// Unsigned multiply, multiply-accumulate instructions.
>>>> +class AI6<bits<4> opcod, dag oops, dag iops, Format f, string opc,
>>>> +         string asm, list<dag> pattern>
>>>> +  : I<opcod, oops, iops, AddrMode6, Size4Bytes, IndexModeNone, f,
>>>> opc,
>>>> +      asm,"",pattern>
>>>> +{
>>>> +  // FIXME: bits 7-4 should be a sub-mode (for SMLAxx,  
>>>> SMLAWy, ...)
>>>> +  let Inst{7-4}   = 0b1001;
>>>> +  let Inst{27-24} = 0b0000;
>>>> +  let Inst{23-20} = opcod;
>>>> +}
>>>> +class AsI6<bits<4> opcod, dag oops, dag iops, Format f, string  
>>>> opc,
>>>> +          string asm, list<dag> pattern>
>>>> +  : sI<opcod, oops, iops, AddrMode6, Size4Bytes, IndexModeNone, f,
>>>> opc,
>>>> +       asm,"",pattern>
>>>> +{
>>>> +  // FIXME: bits 7-4 should be a sub-mode (for SMLAxx,  
>>>> SMLAWy, ...)
>>>> +  let Inst{7-4}   = 0b1001;
>>>> +  let Inst{27-24} = 0b0000;
>>>> +  let Inst{23-20} = opcod;
>>>> +}
>>>>
>>>> //
>>>> =
>>>> =
>>>> =
>>>> ----------------------------------------------------------------------=
>>>> ==//
>>>>
>>>>
>>>> Modified: llvm/trunk/lib/Target/ARM/ARMInstrInfo.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.h?rev=58626&r1=58625&r2=58626&view=diff
>>>>
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> = 
>>>> ===================================================================
>>>> --- llvm/trunk/lib/Target/ARM/ARMInstrInfo.h (original)
>>>> +++ llvm/trunk/lib/Target/ARM/ARMInstrInfo.h Mon Nov  3 12:38:31
>>>> 2008
>>>> @@ -30,8 +30,7 @@
>>>>  // Instruction Flags.
>>>>
>>>>  //
>>>> =
>>>> = 
>>>> =------------------------------------------------------------------
>>>> ===//
>>>> -    // This three-bit field describes the addressing mode used.
>>>> Zero is unused
>>>> -    // so that we can tell if we forgot to set a value.
>>>> +    // This four-bit field describes the addressing mode used.
>>>>
>>>>  AddrModeMask  = 0xf,
>>>>  AddrModeNone  = 0,
>>>> @@ -40,10 +39,11 @@
>>>>  AddrMode3     = 3,
>>>>  AddrMode4     = 4,
>>>>  AddrMode5     = 5,
>>>> -    AddrModeT1    = 6,
>>>> -    AddrModeT2    = 7,
>>>> -    AddrModeT4    = 8,
>>>> -    AddrModeTs    = 9,   // i8 * 4 for pc and sp relative data
>>>> +    AddrMode6     = 6,
>>>> +    AddrModeT1    = 7,
>>>> +    AddrModeT2    = 8,
>>>> +    AddrModeT4    = 9,
>>>> +    AddrModeTs    = 10,  // i8 * 4 for pc and sp relative data
>>>>
>>>>  // Size* - Flags to keep track of the size of an instruction.
>>>>  SizeShift     = 4,
>>>> @@ -115,15 +115,17 @@
>>>>
>>>>  // Field shifts - such shifts are used to set field while
>>>> generating
>>>>  // machine instructions.
>>>> -    RotImmShift = 8,
>>>> -    RegRsShift  = 8,
>>>> -    RegRdShift  = 12,
>>>> -    RegRnShift  = 16,
>>>> -    L_BitShift  = 20,
>>>> -    S_BitShift  = 20,
>>>> -    U_BitShift  = 23,
>>>> -    IndexShift  = 24,
>>>> -    I_BitShift  = 25
>>>> +    RotImmShift  = 8,
>>>> +    RegRsShift   = 8,
>>>> +    RegRdLoShift = 12,
>>>> +    RegRdShift   = 12,
>>>> +    RegRdHiShift = 16,
>>>> +    RegRnShift   = 16,
>>>> +    L_BitShift   = 20,
>>>> +    S_BitShift   = 20,
>>>> +    U_BitShift   = 23,
>>>> +    IndexShift   = 24,
>>>> +    I_BitShift   = 25
>>>> };
>>>> }
>>>>
>>>>
>>>> Modified: llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.td?rev=58626&r1=58625&r2=58626&view=diff
>>>>
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> = 
>>>> ===================================================================
>>>> --- llvm/trunk/lib/Target/ARM/ARMInstrInfo.td (original)
>>>> +++ llvm/trunk/lib/Target/ARM/ARMInstrInfo.td Mon Nov  3 12:38:31
>>>> 2008
>>>> @@ -310,10 +310,11 @@
>>>> def AddrMode3    : AddrMode<3>;
>>>> def AddrMode4    : AddrMode<4>;
>>>> def AddrMode5    : AddrMode<5>;
>>>> -def AddrModeT1   : AddrMode<6>;
>>>> -def AddrModeT2   : AddrMode<7>;
>>>> -def AddrModeT4   : AddrMode<8>;
>>>> -def AddrModeTs   : AddrMode<9>;
>>>> +def AddrMode6    : AddrMode<6>;
>>>> +def AddrModeT1   : AddrMode<7>;
>>>> +def AddrModeT2   : AddrMode<8>;
>>>> +def AddrModeT4   : AddrMode<9>;
>>>> +def AddrModeTs   : AddrMode<10>;
>>>>
>>>> // Instruction size.
>>>> class SizeFlagVal<bits<3> val> {
>>>> @@ -910,49 +911,53 @@
>>>> //  Multiply Instructions.
>>>> //
>>>>
>>>> -def MUL  : AsI<0x0, (outs GPR:$dst), (ins GPR:$a, GPR:$b), MulFrm,
>>>> -               "mul", " $dst, $a, $b",
>>>> -               [(set GPR:$dst, (mul GPR:$a, GPR:$b))]>;
>>>> -
>>>> -def MLA  : AsI<0x2, (outs GPR:$dst), (ins GPR:$a, GPR:$b, GPR:$c),
>>>> -               MulFrm, "mla", " $dst, $a, $b, $c",
>>>> -               [(set GPR:$dst, (add (mul GPR:$a, GPR:$b), GPR:
>>>> $c))]>;
>>>> +def MUL   : AsI6<0b0000, (outs GPR:$dst), (ins GPR:$a, GPR:$b),
>>>> MulFrm,
>>>> +                 "mul", " $dst, $a, $b",
>>>> +                 [(set GPR:$dst, (mul GPR:$a, GPR:$b))]>;
>>>> +
>>>> +def MLA   : AsI6<0b0010, (outs GPR:$dst), (ins GPR:$a, GPR:$b,  
>>>> GPR:
>>>> $c),
>>>> +                 MulFrm, "mla", " $dst, $a, $b, $c",
>>>> +                 [(set GPR:$dst, (add (mul GPR:$a, GPR:$b), GPR:
>>>> $c))]>;
>>>>
>>>> // Extra precision multiplies with low / high results
>>>> -def SMULL : AsI<0xC, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,  
>>>> GPR:
>>>> $b),
>>>> -                MulFrm, "smull", " $ldst, $hdst, $a, $b", []>;
>>>> +def SMULL : AsI6<0b1100, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,
>>>> GPR:$b),
>>>> +                 MulFrm, "smull", " $ldst, $hdst, $a, $b", []>;
>>>>
>>>> -def UMULL : AsI<0x8, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,  
>>>> GPR:
>>>> $b),
>>>> -                MulFrm, "umull", " $ldst, $hdst, $a, $b", []>;
>>>> +def UMULL : AsI6<0b1000, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,
>>>> GPR:$b),
>>>> +                 MulFrm, "umull", " $ldst, $hdst, $a, $b", []>;
>>>>
>>>> // Multiply + accumulate
>>>> -def SMLAL : AsI<0xE, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,  
>>>> GPR:
>>>> $b),
>>>> -                MulFrm, "smlal", " $ldst, $hdst, $a, $b", []>;
>>>> +def SMLAL : AsI6<0b1110, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,
>>>> GPR:$b),
>>>> +                 MulFrm, "smlal", " $ldst, $hdst, $a, $b", []>;
>>>>
>>>> -def UMLAL : AsI<0xA, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,  
>>>> GPR:
>>>> $b),
>>>> -                MulFrm, "umlal", " $ldst, $hdst, $a, $b", []>;
>>>> +def UMLAL : AsI6<0b1010, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,
>>>> GPR:$b),
>>>> +                 MulFrm, "umlal", " $ldst, $hdst, $a, $b", []>;
>>>>
>>>> -def UMAAL : AI<0x0, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a, GPR:
>>>> $b), MulFrm,
>>>> -               "umaal", " $ldst, $hdst, $a, $b", []>,
>>>> -            Requires<[IsARM, HasV6]>;
>>>> +def UMAAL : AI6 <0b0000, (outs GPR:$ldst, GPR:$hdst), (ins GPR:$a,
>>>> GPR:$b),
>>>> +                 MulFrm, "umaal", " $ldst, $hdst, $a, $b", []>,
>>>> +                 Requires<[IsARM, HasV6]>;
>>>>
>>>> // Most significant word multiply
>>>> +// FIXME: encoding
>>>> def SMMUL : AI<0x0, (outs GPR:$dst), (ins GPR:$a, GPR:$b), MulFrm,
>>>>             "smmul", " $dst, $a, $b",
>>>>             [(set GPR:$dst, (mulhs GPR:$a, GPR:$b))]>,
>>>>          Requires<[IsARM, HasV6]>;
>>>>
>>>> +// FIXME: encoding
>>>> def SMMLA : AI<0x0, (outs GPR:$dst), (ins GPR:$a, GPR:$b, GPR:$c),
>>>> MulFrm,
>>>>             "smmla", " $dst, $a, $b, $c",
>>>>             [(set GPR:$dst, (add (mulhs GPR:$a, GPR:$b), GPR:
>>>> $c))]>,
>>>>          Requires<[IsARM, HasV6]>;
>>>>
>>>>
>>>> +// FIXME: encoding
>>>> def SMMLS : AI<0x0, (outs GPR:$dst), (ins GPR:$a, GPR:$b, GPR:$c),
>>>> MulFrm,
>>>>             "smmls", " $dst, $a, $b, $c",
>>>>             [(set GPR:$dst, (sub GPR:$c, (mulhs GPR:$a, GPR:
>>>> $b)))]>,
>>>>             Requires<[IsARM, HasV6]>;
>>>>
>>>> +// FIXME: encoding
>>>> multiclass AI_smul<string opc, PatFrag opnode> {
>>>> def BB : AI<0x8, (outs GPR:$dst), (ins GPR:$a, GPR:$b), MulSMUL,
>>>>            !strconcat(opc, "bb"), " $dst, $a, $b",
>>>> @@ -992,6 +997,7 @@
>>>> }
>>>>
>>>>
>>>> +// FIXME: encoding
>>>> multiclass AI_smla<string opc, PatFrag opnode> {
>>>> def BB : AI<0x8, (outs GPR:$dst), (ins GPR:$a, GPR:$b, GPR:$acc),
>>>> MulSMLA,
>>>>            !strconcat(opc, "bb"), " $dst, $a, $b, $acc",
>>>> @@ -1031,7 +1037,9 @@
>>>>          Requires<[IsARM, HasV5TE]>;
>>>> }
>>>>
>>>> +// FIXME: encoding
>>>> defm SMUL : AI_smul<"smul", BinOpFrag<(mul node:$LHS, node:$RHS)>>;
>>>> +// FIXME: encoding
>>>> defm SMLA : AI_smla<"smla", BinOpFrag<(mul node:$LHS, node:$RHS)>>;
>>>>
>>>> // TODO: Halfword multiple accumulate long: SMLAL<x><y>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
>>
>> _______________________________________________
>> 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