[Kryo] Use immediate #0 to zero a register

[haicheng]

Summary

Similar to Cyclone, Kryo favors using immediate #0 to zero out registers.

[mcrosier]

I'd replace "HasImmeZeroing" with "UseImmZeroing".

lib/Target/AArch64/AArch64.td

52

s/imme/imm

54

s/great/preferred

56

s/imme/imm

57

s/immez/immz

151

s/Imme/Imm

lib/Target/AArch64/AArch64Subtarget.h

139 ↗

(On Diff #56309)

s/has/use

[haicheng]

Change one line of the comment.

[haicheng]

Addresses Chad's comments.

[mcrosier]

@t.p.northover: Much like Cyclone, Kryo prefers the move immediate zero idiom for zeroing registers. Unfortunately, when we tried to reuse the ZeroCycleZeroing target feature we found regressions in some of our FP workloads. The issue is described in PR27454. Do you see a way we could have a unified approach for Cyclone and Kryo that avoids the issue in PR27454?

lib/Target/AArch64/AArch64InstrInfo.cpp

1940

If I'm not mistaken, Matt suggested this be Subtarget.isKryo(), rather than adding a new target feature.

[haicheng]

Change to use subtarget hook. Enable float point zeroing as well.

[haicheng]

Now that r274686 is committed, I can simply enable zczeroing feature in Kryo for both integer and float registers. I have to explicitly set the isAsCheapAsAMove feature because Kryo has customized CheapAsMoveHandling.

[MatzeB]

Looks good to me, but I believe the test can be simplified:

test/CodeGen/AArch64/kryo_zeroing.ll

26–45

These tests appear to have a lot more instructions than necessary just to produce mov x9, #0; mov x8, #0. Also wouldn't it be better to merge this with arm64-zero-cycle-zeroing.ll, maybe you can just addnother run line there?

[haicheng]

As suggested by Matthias, add another run line in arm64-zero-cycle-zeroing.ll

[MatzeB]

lib/Target/AArch64/AArch64InstrInfo.cpp

629–632

Shouldn't this rather be "case AArch64::COPY: return true"?

The whole purpose of this function is that some passes can decide whether to copy a value around with COPY or just recompute the value instead.

[haicheng]

lib/Target/AArch64/AArch64InstrInfo.cpp

629–632

I agree with you. I don't know why COPY is not included in the first place. However, can I do it in a separate patch because it affects other subtargets? This patch is supposed to affect Kryo only.

[haicheng]

I separate this patch into two parts, ZCZeroing part and isAsCheapAsAMove part. This patch now only has the first part.

I am still collecting performance data of setting any COPY isAsCheapAsAMove on A57 and Kryo which may takes a while. Moreover, I am thinking maybe I should use isZeroCost() API to set all zero cost pseudo-instructions (including COPY) isAsCheapAsAMove. I will upload another patch after the experiment is done.

[MatzeB]

LGTM.

[haicheng]

Thank you, Matthias.

Another thing about Cyclone's ZCZeroing feature is AArch64InstrInfo::isGPRZero() which I believe is only used by Cyclone. This function returns true for COPY WZR but not COPY XZR. I don't know if it is intentional or a mistake.

lib/Target/AArch64/AArch64.td

 def FeatureSPE : SubtargetFeature<"spe", "HasSPE", "true",
   "Enable Statistical Profiling extension">;
 
 /// Cyclone has register move instructions which are "free".
 def FeatureZCRegMove : SubtargetFeature<"zcm", "HasZeroCycleRegMove", "true",
                                         "Has zero-cycle register moves">;
 
 /// Cyclone has instructions which zero registers for "free".
 def FeatureZCZeroing : SubtargetFeature<"zcz", "HasZeroCycleZeroing", "true",
                                         "Has zero-cycle zeroing instructions">;
 
 def FeatureStrictAlign : SubtargetFeature<"strict-align",
                                           "StrictAlign", "true",
                                           "Disallow all unaligned memory "
                                           "access">;
 
 def FeatureReserveX18 : SubtargetFeature<"reserve-x18", "ReserveX18", "true",
                                          "Reserve X18, making it unavailable "
                                          "as a GPR">;
 
 def FeatureMergeNarrowLd : SubtargetFeature<"merge-narrow-ld",
                                             "MergeNarrowLoads", "true",

 include "AArch64SchedA53.td"
 include "AArch64SchedA57.td"
 include "AArch64SchedCyclone.td"
 include "AArch64SchedM1.td"
 include "AArch64SchedKryo.td"
 include "AArch64SchedVulcan.td"
 
 def ProcA35     : SubtargetFeature<"a35", "ARMProcFamily", "CortexA35",
                                    "Cortex-A35 ARM processors", [
                                    FeatureCRC,
                                    FeatureCrypto,
                                    FeatureFPARMv8,
                                    FeatureNEON,
                                    FeaturePerfMon
                                    ]>;
 
 def ProcA53     : SubtargetFeature<"a53", "ARMProcFamily", "CortexA53",
def ProcKryo    : SubtargetFeature<"kryo", "ARMProcFamily", "Kryo",
                                    FeatureCRC,
                                    FeatureCrypto,
                                    FeatureCustomCheapAsMoveHandling,
                                    FeatureFPARMv8,
                                    FeatureMergeNarrowLd,
                                    FeatureNEON,
                                    FeaturePerfMon,
                                    FeaturePostRAScheduler,
-                                   FeaturePredictableSelectIsExpensive
+                                   FeaturePredictableSelectIsExpensive,
+                                   FeatureZCZeroing
                                    ]>;
 
 def ProcVulcan  : SubtargetFeature<"vulcan", "ARMProcFamily", "Vulcan",
                                    "Broadcom Vulcan processors", [
                                    FeatureCRC,
                                    FeatureCrypto,
                                    FeatureFPARMv8,
                                    FeatureNEON,

lib/Target/AArch64/AArch64InstrInfo.cpp

return (Subtarget.getProcFamily() == AArch64Subtarget::ExynosM1 &&
             AArch64_AM::getShiftType(Imm) == AArch64_AM::LSL);
 
   // If MOVi32imm or MOVi64imm can be expanded into ORRWri or
   // ORRXri, it is as cheap as MOV
   case AArch64::MOVi32imm:
     return canBeExpandedToORR(MI, 32);
   case AArch64::MOVi64imm:
     return canBeExpandedToORR(MI, 64);
+
+  // Kryo has zero cycle zeroing feature.
+  case AArch64::FMOVS0:
+  case AArch64::FMOVD0:
+    return (Subtarget.getProcFamily() == AArch64Subtarget::Kryo);
+
+  case AArch64::COPY:
+    return (Subtarget.getProcFamily() == AArch64Subtarget::Kryo &&
+            (MI.getOperand(1).getReg() == AArch64::WZR ||
+             MI.getOperand(1).getReg() == AArch64::XZR));
   }
 
   llvm_unreachable("Unknown opcode to check as cheap as a move!");
 }
 
 bool AArch64InstrInfo::isCoalescableExtInstr(const MachineInstr &MI,
                                              unsigned &SrcReg, unsigned &DstReg,
                                              unsigned &SubIdx) const {
if (DestReg == AArch64::WSP || SrcReg == AArch64::WSP) {
             .addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
       } else {
         BuildMI(MBB, I, DL, get(AArch64::ADDWri), DestReg)
             .addReg(SrcReg, getKillRegState(KillSrc))
             .addImm(0)
             .addImm(AArch64_AM::getShifterImm(AArch64_AM::LSL, 0));
       }
     } else if (SrcReg == AArch64::WZR && Subtarget.hasZeroCycleZeroing()) {
       BuildMI(MBB, I, DL, get(AArch64::MOVZWi), DestReg).addImm(0).addImm(
           AArch64_AM::getShifterImm(AArch64_AM::LSL, 0));
     } else {
       if (Subtarget.hasZeroCycleRegMove()) {
         // Cyclone recognizes "ORR Xd, XZR, Xm" as a zero-cycle register move.
         unsigned DestRegX = TRI->getMatchingSuperReg(DestReg, AArch64::sub_32,
                                                      &AArch64::GPR64spRegClass);
         unsigned SrcRegX = TRI->getMatchingSuperReg(SrcReg, AArch64::sub_32,
                                                     &AArch64::GPR64spRegClass);

test/CodeGen/AArch64/kryo_zeroing.ll

+; RUN: llc -mtriple=aarch64-gnu-linux -mcpu=kryo < %s | FileCheck %s
+
+; Verify that immediate #0 is used when zeroing a D, X, W register.
+ 
+define i32 @test_int32(i32* nocapture readonly %p) {
+; CHECK-LABEL:  test_int32:
+; CHECK:  mov  x9, #0
+; CHECK:  mov  w8, #0
+entry:
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body
+  ret i32 %add
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %sum.06 = phi i32 [ 0, %entry ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %p, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx, align 4
+  %add = add nsw i32 %0, %sum.06
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+}
+
+define i64 @test_int64(i64* nocapture readonly %p) {
+; CHECK-LABEL:  test_int64:
+; CHECK:  mov  x9, #0
+; CHECK:  mov  x8, #0
+entry:
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body
+  ret i64 %add
+
+for.body:                                         ; preds = %for.body, %entry
+  %i.07 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
+  %sum.06 = phi i64 [ 0, %entry ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds i64, i64* %p, i64 %i.07
+  %0 = load i64, i64* %arrayidx, align 8
+  %add = add nsw i64 %0, %sum.06
+  %inc = add nuw nsw i64 %i.07, 1
+  %exitcond = icmp eq i64 %inc, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+}
+
+define double @test_double(double* nocapture readonly %p) {
+; CHECK-LABEL:  test_double:
+; CHECK:  movi   v0.2d, #0000000000000000
+; CHECK:  mov  x8, #0
+entry:
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body
+  ret double %add
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %sum.06 = phi double [ 0.0, %entry ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds double, double* %p, i64 %indvars.iv
+  %0 = load double, double* %arrayidx, align 4
+  %add = fadd double %0, %sum.06
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+}
+
+define float @test_float(float* nocapture readonly %p) {
+; CHECK-LABEL:  test_float:
+; CHECK:  movi   v0.2d, #0000000000000000
+; CHECK:  mov  x8, #0
+entry:
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body
+  ret float %add
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %sum.06 = phi float [ 0.0, %entry ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds float, float* %p, i64 %indvars.iv
+  %0 = load float, float* %arrayidx, align 4
+  %add = fadd float %0, %sum.06
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+}