[llvm][sve] Reg + Imm addressing mode for ld1ro.

[sdesmalen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

12229

This change doesn't seem to be tested. Does this belong in a separate patch?

llvm/lib/Target/AArch64/SVEInstrFormats.td

7725

A few questions:

• Should this pattern also use (PredTy PPR3bAny:$gp)?
• Why 2 in AddedComplexity = 2 ?
• Is AddedComplexity needed at all? (it already looks more 'complex' than the other pattern)

nit: i don't know why gp is used here instead of Pg, which is used in the rest of file.

[fpetrogalli]

Rename variables in tablegen patterns.

[fpetrogalli]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

12229

This makes sure that the bfloat case in the test file will work only if the +bf16 is added to the target feature. If I remove this, the bfloat case will lower even without the +bf16 target feature.

To properly test this, I shall create an llc test that fails with a when +bf16 is not listed as a target feature and that catches the "missing patters" debug message from the failure. I don't think we have tests like these, but let me know if this is something you want me to add one.

FWIW, this "early exit if lowering bfloat when not targeting bfloat" code is used also in other places, like performST1Combine. It is needed because we have encoded only integer patterns in the tablegen files, and we lower FP values by bitcasting them to integer values.

llvm/lib/Target/AArch64/SVEInstrFormats.td

7725

Should this pattern also use (PredTy PPR3bAny:$gp)

Done.

Why 2 in AddedComplexity = 2 ?

I don't know, but neither no added complexity or AddedComplexity = 1 work. In both cases, the reg+reg addressing mode specified in the multicalss sve_mem_ldor_ss wins. AddedComplexity = 2 is the first non-zero value that makes it work...

nit: i don't know why gp is used here instead of Pg, which is used in the rest of file.

I made the class use $Pg. I guess I got $gp by copy-paste from other places (I can see few in this file).

[sdesmalen]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

12229

Ah okay, so it is kind of tested, i.e. when removing the +bf16 attribute the test would now fail, where it didn't previously. I don't think a special negative test is needed though.

12230

I think this should be cast, not static_cast.

llvm/lib/Target/AArch64/SVEInstrFormats.td

7725

Okay thanks for clarifying and fixing.

7726

nit: odd indentation.

[fpetrogalli]

Thank you for the review @sdesmalen, I have addressed all your comments.

Francesco

[fpetrogalli]

Adding some text here as phab doesn't allow me to submit empty comments!

[sdesmalen]

LGTM, thanks!

[fpetrogalli]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

12230

reverting to static_cast as cast breaks the build

[fpetrogalli]

I replaced cast with static_cast to fix build failures.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

 
 template <unsigned Opcode>
 static SDValue performLD1ReplicateCombine(SDNode *N, SelectionDAG &DAG) {
   static_assert(Opcode == AArch64ISD::LD1RQ_MERGE_ZERO ||
                     Opcode == AArch64ISD::LD1RO_MERGE_ZERO,
                 "Unsupported opcode.");
   SDLoc DL(N);
   EVT VT = N->getValueType(0);
+  if (VT == MVT::nxv8bf16 &&
+      !static_cast<const AArch64Subtarget &>(DAG.getSubtarget()).hasBF16())
+    return SDValue();
 
   EVT LoadVT = VT;
   if (VT.isFloatingPoint())
     LoadVT = VT.changeTypeToInteger();
 
   SDValue Ops[] = {N->getOperand(0), N->getOperand(2), N->getOperand(3)};
   SDValue Load = DAG.getNode(Opcode, DL, {LoadVT, MVT::Other}, Ops);
   SDValue LoadChain = SDValue(Load.getNode(), 1);

llvm/lib/Target/AArch64/AArch64InstrFormats.td

def SImmS3XForm : SDNodeXForm<imm, [{
   return CurDAG->getTargetConstant(N->getSExtValue() / 3, SDLoc(N), MVT::i64);
 }]>;
 def SImmS4XForm : SDNodeXForm<imm, [{
   return CurDAG->getTargetConstant(N->getSExtValue() / 4, SDLoc(N), MVT::i64);
 }]>;
 def SImmS16XForm : SDNodeXForm<imm, [{
   return CurDAG->getTargetConstant(N->getSExtValue() / 16, SDLoc(N), MVT::i64);
 }]>;
+def SImmS32XForm : SDNodeXForm<imm, [{
+  return CurDAG->getTargetConstant(N->getSExtValue() / 32, SDLoc(N), MVT::i64);
+}]>;
 
 // simm6sN predicate - True if the immediate is a multiple of N in the range
 // [-32 * N, 31 * N].
 def SImm6s1Operand : SImmScaledMemoryIndexed<6, 1>;
 def simm6s1 : Operand<i64>, ImmLeaf<i64, [{ return Imm >= -32 && Imm < 32; }]> {
   let ParserMatchClass = SImm6s1Operand;
   let DecoderMethod = "DecodeSImm<6>";
 }

 }
 def simm4s16 : Operand<i64>, ImmLeaf<i64,
 [{ return Imm >=-128  && Imm <= 112 && (Imm % 16) == 0x0; }], SImmS16XForm> {
   let PrintMethod = "printImmScale<16>";
   let ParserMatchClass = SImm4s16Operand;
   let DecoderMethod = "DecodeSImm<4>";
 }
 def simm4s32 : Operand<i64>, ImmLeaf<i64,
-[{ return Imm >=-256  && Imm <= 224 && (Imm % 32) == 0x0; }]> {
+[{ return Imm >=-256  && Imm <= 224 && (Imm % 32) == 0x0; }], SImmS32XForm> {
   let PrintMethod = "printImmScale<32>";
   let ParserMatchClass = SImm4s32Operand;
   let DecoderMethod = "DecodeSImm<4>";
 }
 
 def Imm1_8Operand : AsmImmRange<1, 8>;
 def Imm1_16Operand : AsmImmRange<1, 16>;
 def Imm1_32Operand : AsmImmRange<1, 32>;

llvm/lib/Target/AArch64/SVEInstrFormats.td

multiclass sve_mem_ldor_si<bits<2> sz, string asm, RegisterOperand listty,
   def : InstAlias<asm # "\t$Zt, $Pg/z, [$Rn]",
                   (!cast<Instruction>(NAME) zprty:$Zt, PPR3bAny:$Pg, GPR64sp:$Rn, 0), 0>;
   def : InstAlias<asm # "\t$Zt, $Pg/z, [$Rn, $imm4]",
                   (!cast<Instruction>(NAME) zprty:$Zt, PPR3bAny:$Pg, GPR64sp:$Rn, simm4s32:$imm4), 0>;
 
   // Base addressing mode
   def : Pat<(Ty (Ld1ro (PredTy PPR3bAny:$gp), GPR64sp:$base)),
             (!cast<Instruction>(NAME) PPR3bAny:$gp, GPR64sp:$base, (i64 0))>;
-
+  let AddedComplexity = 2 in {
+  // Reg + Imm addressing mode
+    def : Pat<(Ty (Ld1ro PPR3bAny:$gp, (add GPR64:$base, (i64 simm4s32:$imm)))),
+              (!cast<Instruction>(NAME) $gp, $base, simm4s32:$imm)>;
+  }
 }
 
 class sve_mem_ldor_ss<bits<2> sz, string asm, RegisterOperand VecList,
                       RegisterOperand gprty>
 : I<(outs VecList:$Zt), (ins PPR3bAny:$Pg, GPR64sp:$Rn, gprty:$Rm),
   asm, "\t$Zt, $Pg/z, [$Rn, $Rm]", "", []>, Sched<[]> {
   bits<5> Zt;
   bits<3> Pg;

llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro-addressing-mode-reg-imm.ll

+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve,+f64mm -asm-verbose=0 < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
+
+;
+; LD1ROB
+;
+
+define <vscale x 16 x i8> @ld1rob_i8(<vscale x 16 x i1> %pg, i8* %a) nounwind {
+; CHECK-LABEL: ld1rob_i8:
+; CHECK-NEXT:  ld1rob { z0.b }, p0/z, [x0, #32]
+; CHECK-NEXT:  ret
+  %base = getelementptr i8, i8* %a, i64 32
+  %load = call <vscale x 16 x i8> @llvm.aarch64.sve.ld1ro.nxv16i8(<vscale x 16 x i1> %pg, i8* %base)
+  ret <vscale x 16 x i8> %load
+}
+
+;
+; LD1ROH
+;
+
+define <vscale x 8 x i16> @ld1roh_i16(<vscale x 8 x i1> %pg, i16* %a) nounwind {
+; CHECK-LABEL: ld1roh_i16:
+; CHECK-NEXT:  ld1roh { z0.h }, p0/z, [x0, #64]
+; CHECK-NEXT:  ret
+  %base = getelementptr i16, i16* %a, i64 32
+  %load = call <vscale x 8 x i16> @llvm.aarch64.sve.ld1ro.nxv8i16(<vscale x 8 x i1> %pg, i16* %base)
+  ret <vscale x 8 x i16> %load
+}
+
+define <vscale x 8 x half> @ld1roh_f16(<vscale x 8 x i1> %pg, half* %a) nounwind {
+; CHECK-LABEL: ld1roh_f16:
+; CHECK-NEXT:  ld1roh { z0.h }, p0/z, [x0, #64]
+; CHECK-NEXT:  ret
+  %base = getelementptr half, half* %a, i64 32
+  %load = call <vscale x 8 x half> @llvm.aarch64.sve.ld1ro.nxv8f16(<vscale x 8 x i1> %pg, half* %base)
+  ret <vscale x 8 x half> %load
+}
+
+define <vscale x 8 x bfloat> @ld1roh_bf16(<vscale x 8 x i1> %pg, bfloat* %a) nounwind #0 {
+; CHECK-LABEL: ld1roh_bf16:
+; CHECK-NEXT:  ld1roh { z0.h }, p0/z, [x0, #64]
+; CHECK-NEXT:  ret
+  %base = getelementptr bfloat, bfloat* %a, i64 32
+  %load = call <vscale x 8 x bfloat> @llvm.aarch64.sve.ld1ro.nxv8bf16(<vscale x 8 x i1> %pg, bfloat* %base)
+  ret <vscale x 8 x bfloat> %load
+}
+
+;
+; LD1ROW
+;
+
+define<vscale x 4 x i32> @ld1row_i32(<vscale x 4 x i1> %pg, i32* %a) nounwind {
+; CHECK-LABEL: ld1row_i32:
+; CHECK-NEXT:  ld1row { z0.s }, p0/z, [x0, #128]
+; CHECK-NEXT:  ret
+  %base = getelementptr i32, i32* %a, i64 32
+  %load = call <vscale x 4 x i32> @llvm.aarch64.sve.ld1ro.nxv4i32(<vscale x 4 x i1> %pg, i32* %base)
+  ret <vscale x 4 x i32> %load
+}
+
+define<vscale x 4 x float> @ld1row_f32(<vscale x 4 x i1> %pg, float* %a) nounwind {
+; CHECK-LABEL: ld1row_f32:
+; CHECK-NEXT:  ld1row { z0.s }, p0/z, [x0, #128]
+; CHECK-NEXT:  ret
+  %base = getelementptr float, float* %a, i64 32
+  %load = call <vscale x 4 x float> @llvm.aarch64.sve.ld1ro.nxv4f32(<vscale x 4 x i1> %pg, float* %base)
+  ret <vscale x 4 x float> %load
+}
+
+;
+; LD1ROD
+;
+
+define <vscale x 2 x i64> @ld1rod_i64(<vscale x 2 x i1> %pg, i64* %a) nounwind {
+; CHECK-LABEL: ld1rod_i64:
+; CHECK-NEXT:  ld1rod { z0.d }, p0/z, [x0, #-64]
+; CHECK-NEXT:  ret
+  %base = getelementptr i64, i64* %a, i64 -8
+  %load = call <vscale x 2 x i64> @llvm.aarch64.sve.ld1ro.nxv2i64(<vscale x 2 x i1> %pg, i64* %base)
+  ret <vscale x 2 x i64> %load
+}
+
+define <vscale x 2 x double> @ld1rod_f64(<vscale x 2 x i1> %pg, double* %a) nounwind {
+; CHECK-LABEL: ld1rod_f64:
+; CHECK-NEXT:  ld1rod { z0.d }, p0/z, [x0, #-128]
+; CHECK-NEXT:  ret
+  %base = getelementptr double, double* %a, i64 -16
+  %load = call <vscale x 2 x double> @llvm.aarch64.sve.ld1ro.nxv2f64(<vscale x 2 x i1> %pg, double* %base)
+  ret <vscale x 2 x double> %load
+}
+
+
+;;;;;;;;;;;;;;
+; range checks: immediate must be a multiple of 32 in the range -256, ..., 224
+
+; lower bound
+define <vscale x 16 x i8> @ld1rob_i8_lower_bound(<vscale x 16 x i1> %pg, i8* %a) nounwind {
+; CHECK-LABEL: ld1rob_i8_lower_bound:
+; CHECK-NEXT:  ld1rob { z0.b }, p0/z, [x0, #-256]
+; CHECK-NEXT:  ret
+  %base = getelementptr i8, i8* %a, i64 -256
+  %load = call <vscale x 16 x i8> @llvm.aarch64.sve.ld1ro.nxv16i8(<vscale x 16 x i1> %pg, i8* %base)
+  ret <vscale x 16 x i8> %load
+}
+
+; below lower bound
+define <vscale x 8 x i16> @ld1roh_i16_below_lower_bound(<vscale x 8 x i1> %pg, i16* %a) nounwind {
+; CHECK-LABEL: ld1roh_i16_below_lower_bound:
+; CHECK-NEXT:  sub     x[[BASE:[0-9]+]], x0, #258
+; CHECK-NEXT:  ld1roh { z0.h }, p0/z, [x[[BASE]]]
+; CHECK-NEXT:  ret
+  %base = getelementptr i16, i16* %a, i64 -129
+  %load = call <vscale x 8 x i16> @llvm.aarch64.sve.ld1ro.nxv8i16(<vscale x 8 x i1> %pg, i16* %base)
+  ret <vscale x 8 x i16> %load
+}
+
+define <vscale x 16 x i8> @ld1rob_i8_below_lower_bound_01(<vscale x 16 x i1> %pg, i8* %a) nounwind {
+; CHECK-LABEL: ld1rob_i8_below_lower_bound_01:
+; CHECK-NEXT:  mov x[[OFFSET:[0-9]+]], #-257
+; CHECK-NEXT:  ld1rob { z0.b }, p0/z, [x0, x[[OFFSET]]]
+; CHECK-NEXT:  ret
+  %base = getelementptr i8, i8* %a, i64 -257
+  %load = call <vscale x 16 x i8> @llvm.aarch64.sve.ld1ro.nxv16i8(<vscale x 16 x i1> %pg, i8* %base)
+  ret <vscale x 16 x i8> %load
+}
+
+; not a multiple of 32
+define<vscale x 4 x i32> @ld1row_i32_not_multiple(<vscale x 4 x i1> %pg, i32* %a) nounwind {
+; CHECK-LABEL: ld1row_i32_not_multiple:
+; CHECK-NEXT:  add x[[BASE:[0-9]+]], x0, #12
+; CHECK-NEXT:  ld1row { z0.s }, p0/z, [x[[BASE]]]
+; CHECK-NEXT:  ret
+  %base = getelementptr i32, i32* %a, i64 3
+  %load = call <vscale x 4 x i32> @llvm.aarch64.sve.ld1ro.nxv4i32(<vscale x 4 x i1> %pg, i32* %base)
+  ret <vscale x 4 x i32> %load
+}
+
+; upper bound
+define <vscale x 2 x i64> @ld1rod_i64_upper_bound(<vscale x 2 x i1> %pg, i64* %a) nounwind {
+; CHECK-LABEL: ld1rod_i64_upper_bound:
+; CHECK-NEXT:  ld1rod { z0.d }, p0/z, [x0, #224]
+; CHECK-NEXT:  ret
+  %base = getelementptr i64, i64* %a, i64 28
+  %load = call <vscale x 2 x i64> @llvm.aarch64.sve.ld1ro.nxv2i64(<vscale x 2 x i1> %pg, i64* %base)
+  ret <vscale x 2 x i64> %load
+}
+
+define <vscale x 16 x i8> @ld1rob_i8_beyond_upper_bound(<vscale x 16 x i1> %pg, i8* %a) nounwind {
+; CHECK-LABEL: ld1rob_i8_beyond_upper_bound:
+; CHECK-NEXT:  mov w[[OFFSET:[0-9]+]], #225
+; CHECK-NEXT:  ld1rob { z0.b }, p0/z, [x0, x[[OFFSET]]]
+; CHECK-NEXT:  ret
+  %base = getelementptr i8, i8* %a, i64 225
+  %load = call <vscale x 16 x i8> @llvm.aarch64.sve.ld1ro.nxv16i8(<vscale x 16 x i1> %pg, i8* %base)
+  ret <vscale x 16 x i8> %load
+}
+
+declare <vscale x 16 x i8> @llvm.aarch64.sve.ld1ro.nxv16i8(<vscale x 16 x i1>, i8*)
+
+declare <vscale x 8 x i16> @llvm.aarch64.sve.ld1ro.nxv8i16(<vscale x 8 x i1>, i16*)
+declare <vscale x 8 x half> @llvm.aarch64.sve.ld1ro.nxv8f16(<vscale x 8 x i1>, half*)
+declare <vscale x 8 x bfloat> @llvm.aarch64.sve.ld1ro.nxv8bf16(<vscale x 8 x i1>, bfloat*)
+
+declare <vscale x 4 x i32> @llvm.aarch64.sve.ld1ro.nxv4i32(<vscale x 4 x i1>, i32*)
+declare <vscale x 4 x float> @llvm.aarch64.sve.ld1ro.nxv4f32(<vscale x 4 x i1>, float*)
+
+declare <vscale x 2 x i64> @llvm.aarch64.sve.ld1ro.nxv2i64(<vscale x 2 x i1>, i64*)
+declare <vscale x 2 x double> @llvm.aarch64.sve.ld1ro.nxv2f64(<vscale x 2 x i1>, double*)
+
+
+; +bf16 is required for the bfloat version.
+attributes #0 = { "target-features"="+sve,+f64mm,+bf16" }