[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 ↗

(On Diff #56309)

s/imme/imm

54 ↗

(On Diff #56309)

s/great/preferred

56 ↗

(On Diff #56309)

s/imme/imm

57 ↗

(On Diff #56309)

s/immez/immz

151 ↗

(On Diff #56309)

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

1932 ↗

(On Diff #56320)

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 ↗

(On Diff #62990)

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 ↗

(On Diff #63008)

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 ↗

(On Diff #63008)

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.

llvm/trunk/lib/Target/AArch64/AArch64.td

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,
                                    FeatureMacroOpFusion,

llvm/trunk/test/CodeGen/AArch64/arm64-zero-cycle-zeroing.ll

-; RUN: llc -mtriple=arm64-apple-ios -mcpu=cyclone < %s | FileCheck %s
+; RUN: llc -mtriple=arm64-apple-ios -mcpu=cyclone < %s | FileCheck %s -check-prefix=CYCLONE --check-prefix=ALL
+; RUN: llc -mtriple=aarch64-gnu-linux -mcpu=kryo < %s | FileCheck %s -check-prefix=KRYO --check-prefix=ALL
+
 ; rdar://11481771
 ; rdar://13713797
 
 define void @t1() nounwind ssp {
 entry:
-; CHECK-LABEL: t1:
-; CHECK-NOT: fmov
-; CHECK: movi.2d v0, #0000000000000000
-; CHECK: movi.2d v1, #0000000000000000
-; CHECK: movi.2d v2, #0000000000000000
-; CHECK: movi.2d v3, #0000000000000000
+; ALL-LABEL: t1:
+; ALL-NOT: fmov
+; CYCLONE: movi.2d v0, #0000000000000000
+; CYCLONE: movi.2d v1, #0000000000000000
+; CYCLONE: movi.2d v2, #0000000000000000
+; CYCLONE: movi.2d v3, #0000000000000000
+; KRYO: movi v0.2d, #0000000000000000
+; KRYO: mov v1.16b, v0.16b 
+; KRYO: mov v2.16b, v0.16b 
+; KRYO: mov v3.16b, v0.16b
   tail call void @bar(double 0.000000e+00, double 0.000000e+00, double 0.000000e+00, double 0.000000e+00) nounwind
   ret void
 }
 
 define void @t2() nounwind ssp {
 entry:
-; CHECK-LABEL: t2:
-; CHECK-NOT: mov w0, wzr
-; CHECK: mov w0, #0
-; CHECK: mov w1, #0
+; ALL-LABEL: t2:
+; ALL-NOT: mov w0, wzr
+; ALL: mov w0, #0
+; ALL: mov w1, #0
   tail call void @bari(i32 0, i32 0) nounwind
   ret void
 }
 
 define void @t3() nounwind ssp {
 entry:
-; CHECK-LABEL: t3:
-; CHECK-NOT: mov x0, xzr
-; CHECK: mov x0, #0
-; CHECK: mov x1, #0
+; ALL-LABEL: t3:
+; ALL-NOT: mov x0, xzr
+; ALL: mov x0, #0
+; ALL: mov x1, #0
   tail call void @barl(i64 0, i64 0) nounwind
   ret void
 }
 
 define void @t4() nounwind ssp {
-; CHECK-LABEL: t4:
-; CHECK-NOT: fmov
-; CHECK: movi.2d v0, #0000000000000000
-; CHECK: movi.2d v1, #0000000000000000
+; ALL-LABEL: t4:
+; ALL-NOT: fmov
+; CYCLONE: movi.2d v0, #0000000000000000
+; CYCLONE: movi.2d v1, #0000000000000000
+; KRYO: movi v0.2d, #0000000000000000
+; KRYO: mov v1.16b, v0.16b
   tail call void @barf(float 0.000000e+00, float 0.000000e+00) nounwind
   ret void
 }
 
 declare void @bar(double, double, double, double)
 declare void @bari(i32, i32)
 declare void @barl(i64, i64)
 declare void @barf(float, float)
 
 ; We used to produce spills+reloads for a Q register with zero cycle zeroing
 ; enabled.
-; CHECK-LABEL: foo:
-; CHECK-NOT: str {{q[0-9]+}}
-; CHECK-NOT: ldr {{q[0-9]+}}
+; ALL-LABEL: foo:
+; ALL-NOT: str {{q[0-9]+}}
+; ALL-NOT: ldr {{q[0-9]+}}
 define double @foo(i32 %n) {
 entry:
   br label %for.body
 
 for.body:
   %phi0 = phi double [ 1.0, %entry ], [ %v0, %for.body ]
   %i.076 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
   %conv21 = sitofp i32 %i.076 to double