[llvm-commits] [PATCH] AArch64 backend: LLVM changes

Tom Stellard tom at stellard.net
Tue Jan 22 07:07:23 PST 2013


Hi Tim,

Just a few things I noticed looking through the code:

On Tue, Jan 22, 2013 at 04:55:57AM -0800, Tim Northover wrote:
> Index: lib/Target/AArch64/AArch64ISelLowering.cpp
> ===================================================================
> --- /dev/null
> +++ lib/Target/AArch64/AArch64ISelLowering.cpp
> @@ -0,0 +1,3424 @@
> +  // Virtually no operation on f128 is legal, but LLVM can't expand them when
> +  // there's a valid register class, so we need custom operations in most cases.
> +  setOperationAction(ISD::FABS,       MVT::f128, Expand);
> +  setOperationAction(ISD::FADD,       MVT::f128, Custom);
> +  setOperationAction(ISD::FCOPYSIGN,  MVT::f128, Expand);
> +  setOperationAction(ISD::FCOS,       MVT::f128, Expand);
> +  setOperationAction(ISD::FDIV,       MVT::f128, Custom);
> +  setOperationAction(ISD::FMA,        MVT::f128, Expand);
> +  setOperationAction(ISD::FMUL,       MVT::f128, Custom);
> +  setOperationAction(ISD::FNEG,       MVT::f128, Expand);
> +  setOperationAction(ISD::FP_EXTEND,  MVT::f128, Expand);
> +  setOperationAction(ISD::FP_ROUND,   MVT::f128, Expand);
> +  setOperationAction(ISD::FPOW,       MVT::f128, Expand);
> +  setOperationAction(ISD::FREM,       MVT::f128, Expand);
> +  setOperationAction(ISD::FRINT,      MVT::f128, Expand);
> +  setOperationAction(ISD::FSIN,       MVT::f128, Expand);
> +  setOperationAction(ISD::FSQRT,      MVT::f128, Expand);
> +  setOperationAction(ISD::FSUB,       MVT::f128, Custom);
> +  setOperationAction(ISD::FTRUNC,     MVT::f128, Expand);
> +  setOperationAction(ISD::SINT_TO_FP, MVT::f128, Custom);
> +  setOperationAction(ISD::UINT_TO_FP, MVT::f128, Custom);

The value type of operand 0 is used to determine the legality of [SU]INT_TO_FP,
nodes, so these last two calls to setOperationAction are unnecessary.

> +  setOperationAction(ISD::SETCC,      MVT::f128, Custom);
> +  setOperationAction(ISD::BR_CC,      MVT::f128, Custom);
> +  setOperationAction(ISD::SELECT,     MVT::f128, Expand);
> +  setOperationAction(ISD::SELECT_CC,  MVT::f128, Custom);
> +  setOperationAction(ISD::FP_EXTEND,  MVT::f128, Custom);
> +
> Index: lib/Target/AArch64/AArch64RegisterInfo.td
> ===================================================================
> --- /dev/null
> +++ lib/Target/AArch64/AArch64RegisterInfo.td
> +//===----------------------------------------------------------------------===//
> +//  Integer registers: w0-w30, wzr, wsp, x0-x30, xzr, sp
> +//===----------------------------------------------------------------------===//
> +
> +def W0  : AArch64Reg< 0, "w0">, DwarfRegNum<[0]>;
> +def W1  : AArch64Reg< 1, "w1">, DwarfRegNum<[1]>;
> +def W2  : AArch64Reg< 2, "w2">, DwarfRegNum<[2]>;
> +def W3  : AArch64Reg< 3, "w3">, DwarfRegNum<[3]>;
> +def W4  : AArch64Reg< 4, "w4">, DwarfRegNum<[4]>;
> +def W5  : AArch64Reg< 5, "w5">, DwarfRegNum<[5]>;
> +def W6  : AArch64Reg< 6, "w6">, DwarfRegNum<[6]>;
> +def W7  : AArch64Reg< 7, "w7">, DwarfRegNum<[7]>;
> +def W8  : AArch64Reg< 8, "w8">, DwarfRegNum<[8]>;
> +def W9  : AArch64Reg< 9, "w9">, DwarfRegNum<[9]>;
> +def W10 : AArch64Reg<10, "w10">, DwarfRegNum<[10]>;
> +def W11 : AArch64Reg<11, "w11">, DwarfRegNum<[11]>;
> +def W12 : AArch64Reg<12, "w12">, DwarfRegNum<[12]>;
> +def W13 : AArch64Reg<13, "w13">, DwarfRegNum<[13]>;
> +def W14 : AArch64Reg<14, "w14">, DwarfRegNum<[14]>;
> +def W15 : AArch64Reg<15, "w15">, DwarfRegNum<[15]>;
> +def W16 : AArch64Reg<16, "w16">, DwarfRegNum<[16]>;
> +def W17 : AArch64Reg<17, "w17">, DwarfRegNum<[17]>;
> +def W18 : AArch64Reg<18, "w18">, DwarfRegNum<[18]>;
> +def W19 : AArch64Reg<19, "w19">, DwarfRegNum<[19]>;
> +def W20 : AArch64Reg<20, "w20">, DwarfRegNum<[20]>;
> +def W21 : AArch64Reg<21, "w21">, DwarfRegNum<[21]>;
> +def W22 : AArch64Reg<22, "w22">, DwarfRegNum<[22]>;
> +def W23 : AArch64Reg<23, "w23">, DwarfRegNum<[23]>;
> +def W24 : AArch64Reg<24, "w24">, DwarfRegNum<[24]>;
> +def W25 : AArch64Reg<25, "w25">, DwarfRegNum<[25]>;
> +def W26 : AArch64Reg<26, "w26">, DwarfRegNum<[26]>;
> +def W27 : AArch64Reg<27, "w27">, DwarfRegNum<[27]>;
> +def W28 : AArch64Reg<28, "w28">, DwarfRegNum<[28]>;
> +def W29 : AArch64Reg<29, "w29">, DwarfRegNum<[29]>;
> +def W30 : AArch64Reg<30, "w30">, DwarfRegNum<[30]>;
> +def WSP : AArch64Reg<31, "wsp">, DwarfRegNum<[31]>;
> +def WZR : AArch64Reg<31, "wzr">;
> +
> +def X0  : AArch64RegWithSubs< 0, "x0", [W0], [sub_32]>, DwarfRegNum<[0]>;
> +def X1  : AArch64RegWithSubs< 1, "x1", [W1], [sub_32]>, DwarfRegNum<[1]>;
> +def X2  : AArch64RegWithSubs< 2, "x2", [W2], [sub_32]>, DwarfRegNum<[2]>;
> +def X3  : AArch64RegWithSubs< 3, "x3", [W3], [sub_32]>, DwarfRegNum<[3]>;
> +def X4  : AArch64RegWithSubs< 4, "x4", [W4], [sub_32]>, DwarfRegNum<[4]>;
> +def X5  : AArch64RegWithSubs< 5, "x5", [W5], [sub_32]>, DwarfRegNum<[5]>;
> +def X6  : AArch64RegWithSubs< 6, "x6", [W6], [sub_32]>, DwarfRegNum<[6]>;
> +def X7  : AArch64RegWithSubs< 7, "x7", [W7], [sub_32]>, DwarfRegNum<[7]>;
> +def X8  : AArch64RegWithSubs< 8, "x8", [W8], [sub_32]>, DwarfRegNum<[8]>;
> +def X9  : AArch64RegWithSubs< 9, "x9", [W9], [sub_32]>, DwarfRegNum<[9]>;
> +def X10 : AArch64RegWithSubs<10, "x10", [W10], [sub_32]>, DwarfRegNum<[10]>;
> +def X11 : AArch64RegWithSubs<11, "x11", [W11], [sub_32]>, DwarfRegNum<[11]>;
> +def X12 : AArch64RegWithSubs<12, "x12", [W12], [sub_32]>, DwarfRegNum<[12]>;
> +def X13 : AArch64RegWithSubs<13, "x13", [W13], [sub_32]>, DwarfRegNum<[13]>;
> +def X14 : AArch64RegWithSubs<14, "x14", [W14], [sub_32]>, DwarfRegNum<[14]>;
> +def X15 : AArch64RegWithSubs<15, "x15", [W15], [sub_32]>, DwarfRegNum<[15]>;
> +def X16 : AArch64RegWithSubs<16, "x16", [W16], [sub_32]>, DwarfRegNum<[16]>;
> +def X17 : AArch64RegWithSubs<17, "x17", [W17], [sub_32]>, DwarfRegNum<[17]>;
> +def X18 : AArch64RegWithSubs<18, "x18", [W18], [sub_32]>, DwarfRegNum<[18]>;
> +def X19 : AArch64RegWithSubs<19, "x19", [W19], [sub_32]>, DwarfRegNum<[19]>;
> +def X20 : AArch64RegWithSubs<20, "x20", [W20], [sub_32]>, DwarfRegNum<[20]>;
> +def X21 : AArch64RegWithSubs<21, "x21", [W21], [sub_32]>, DwarfRegNum<[21]>;
> +def X22 : AArch64RegWithSubs<22, "x22", [W22], [sub_32]>, DwarfRegNum<[22]>;
> +def X23 : AArch64RegWithSubs<23, "x23", [W23], [sub_32]>, DwarfRegNum<[23]>;
> +def X24 : AArch64RegWithSubs<24, "x24", [W24], [sub_32]>, DwarfRegNum<[24]>;
> +def X25 : AArch64RegWithSubs<25, "x25", [W25], [sub_32]>, DwarfRegNum<[25]>;
> +def X26 : AArch64RegWithSubs<26, "x26", [W26], [sub_32]>, DwarfRegNum<[26]>;
> +def X27 : AArch64RegWithSubs<27, "x27", [W27], [sub_32]>, DwarfRegNum<[27]>;
> +def X28 : AArch64RegWithSubs<28, "x28", [W28], [sub_32]>, DwarfRegNum<[28]>;
> +def X29 : AArch64RegWithSubs<29, "x29", [W29], [sub_32]>, DwarfRegNum<[29]>;
> +def X30 : AArch64RegWithSubs<30, "x30", [W30], [sub_32]>, DwarfRegNum<[30]>;
> +def XSP : AArch64RegWithSubs<31, "sp", [WSP], [sub_32]>, DwarfRegNum<[31]>;
> +def XZR : AArch64RegWithSubs<31, "xzr", [WZR], [sub_32]>;
> +

You can use tablegen loops to simplify these kinds of definitions, for example:

foreach Index = 0-30 in {
  def W#Index : AArch64Reg<Index, "w"#Index>, DwarfRegNum<[Index]>;
  // I'm not sure if tablegen will accept [W#Index], you may have to create
  // a string and then cast it as a register.
  def X#Index  : AArch64RegWithSubs< Index, "x"#Index, [W#Index], [sub_32]>, DwarfRegNum<[Index]>;

}

There are several examples using tablegen loops in Target/R600/R600RegisterInfo.td

> +// Most instructions treat register 31 as zero for reads and a black-hole for
> +// writes.
> +
> +// Note that the order of registers is important for the Disassembler here:
> +// tablegen uses it to form MCRegisterClass::getRegister, which we assume can
> +// take an encoding value.
> +def GPR32 : RegisterClass<"AArch64", [i32], 32,
> +                          (add (sequence "W%u", 0, 30), WZR)> {
> +}
> +
> +def GPR64 : RegisterClass<"AArch64", [i64], 64,
> +                          (add (sequence "X%u", 0, 30), XZR)> {
> +}
> +
> +def GPR32nowzr : RegisterClass<"AArch64", [i32], 32,
> +                               (sequence "W%u", 0, 30)> {
> +}
> +
> +def GPR64noxzr : RegisterClass<"AArch64", [i64], 64,
> +                               (sequence "X%u", 0, 30)> {
> +}
> +
> +// For tail calls, we can't use callee-saved registers or the structure-return
> +// register, as they are supposed to be live across function calls and may be
> +// clobbered by the epilogue.
> +def tcGPR64 : RegisterClass<"AArch64", [i64], 64,
> +                            (add (sequence "X%u", 0, 7),
> +                                 (sequence "X%u", 9, 18))> {
> +}
> +
> +
> +// Certain addressing-useful instructions accept sp directly. Again the order of
> +// registers is important to the Disassembler.
> +def GPR32wsp : RegisterClass<"AArch64", [i32], 32,
> +                             (add (sequence "W%u", 0, 30), WSP)> {
> +}
> +
> +def GPR64xsp : RegisterClass<"AArch64", [i64], 64,
> +                             (add (sequence "X%u", 0, 30), XSP)> {
> +}
> +
> +// Some aliases *only* apply to SP (e.g. MOV uses different encoding for SP and
> +// non-SP variants). We can't use a bare register in those patterns because
> +// TableGen doesn't like it, so we need a class containing just stack registers
> +def Rxsp : RegisterClass<"AArch64", [i64], 64,
> +                         (add XSP)> {
> +}
> +
> +def Rwsp : RegisterClass<"AArch64", [i32], 32,
> +                         (add WSP)> {
> +}
> +
> +//===----------------------------------------------------------------------===//
> +//  Scalar registers in the vector unit:
> +//  b0-b31, h0-h31, s0-s31, d0-d31, q0-q31
> +//===----------------------------------------------------------------------===//
> +
> +
> +def B0  : AArch64Reg< 0, "b0">,  DwarfRegNum<[64]>;
> +def B1  : AArch64Reg< 1, "b1">,  DwarfRegNum<[65]>;
> +def B2  : AArch64Reg< 2, "b2">,  DwarfRegNum<[66]>;
> +def B3  : AArch64Reg< 3, "b3">,  DwarfRegNum<[67]>;
> +def B4  : AArch64Reg< 4, "b4">,  DwarfRegNum<[68]>;
> +def B5  : AArch64Reg< 5, "b5">,  DwarfRegNum<[69]>;
> +def B6  : AArch64Reg< 6, "b6">,  DwarfRegNum<[70]>;
> +def B7  : AArch64Reg< 7, "b7">,  DwarfRegNum<[71]>;
> +def B8  : AArch64Reg< 8, "b8">,  DwarfRegNum<[72]>;
> +def B9  : AArch64Reg< 9, "b9">,  DwarfRegNum<[73]>;
> +def B10 : AArch64Reg<10, "b10">, DwarfRegNum<[74]>;
> +def B11 : AArch64Reg<11, "b11">, DwarfRegNum<[75]>;
> +def B12 : AArch64Reg<12, "b12">, DwarfRegNum<[76]>;
> +def B13 : AArch64Reg<13, "b13">, DwarfRegNum<[77]>;
> +def B14 : AArch64Reg<14, "b14">, DwarfRegNum<[78]>;
> +def B15 : AArch64Reg<15, "b15">, DwarfRegNum<[79]>;
> +def B16 : AArch64Reg<16, "b16">, DwarfRegNum<[80]>;
> +def B17 : AArch64Reg<17, "b17">, DwarfRegNum<[81]>;
> +def B18 : AArch64Reg<18, "b18">, DwarfRegNum<[82]>;
> +def B19 : AArch64Reg<19, "b19">, DwarfRegNum<[83]>;
> +def B20 : AArch64Reg<20, "b20">, DwarfRegNum<[84]>;
> +def B21 : AArch64Reg<21, "b21">, DwarfRegNum<[85]>;
> +def B22 : AArch64Reg<22, "b22">, DwarfRegNum<[86]>;
> +def B23 : AArch64Reg<23, "b23">, DwarfRegNum<[87]>;
> +def B24 : AArch64Reg<24, "b24">, DwarfRegNum<[88]>;
> +def B25 : AArch64Reg<25, "b25">, DwarfRegNum<[89]>;
> +def B26 : AArch64Reg<26, "b26">, DwarfRegNum<[90]>;
> +def B27 : AArch64Reg<27, "b27">, DwarfRegNum<[91]>;
> +def B28 : AArch64Reg<28, "b28">, DwarfRegNum<[92]>;
> +def B29 : AArch64Reg<29, "b29">, DwarfRegNum<[93]>;
> +def B30 : AArch64Reg<30, "b30">, DwarfRegNum<[94]>;
> +def B31 : AArch64Reg<31, "b31">, DwarfRegNum<[95]>;
> +

You may be able to represent these with loops too, but you would have to figure
out a way to compute the DwarfRegNum in the loop.

I wasn't able to review all the code, but overall from an organization and
style perspective, it looks pretty good.

-Tom




More information about the llvm-commits mailing list