[AArch64] Lower READCYCLECOUNTER using MRS CNTVCT_EL0

[salvatoredipietro]

Summary

As suggested in D12425 it would be better for the readcyclecounter function on ARM architecture to use the CNTVCT_EL0 register (Counter-timer Virtual Count register) instead of the PMCCNTR_EL0 (Performance Monitors Cycle Count Register) because the PMCCNTR_EL0 is a PMU register which, depending on the configuration, it might always return zeroes and it doesn't guaranteed to always be increased.

[sebpop]

Hi Salvatore,
the patch looks good to me.
As you suggested to me, we should also remove this part let Predicates = [HasPerfMon] in.

A little bit more context on what this patch fixes: MariaDB compiled with clang crashes on arm64-freebsd https://jira.mariadb.org/browse/MDEV-23249

[salvatoredipietro]

Hi Sebastian,
have update the diff removing the predicate for the READCYCLECOUNTER function

[sebpop]

The change looks good to me.
Could one of the arm64 maintainers also approve this patch? Thanks!

[salvatoredipietro]

Thank @sebpop for approving it.
Since I do not have commit access and if there is no other objections, Can you please commit the change on my behalf? Thanks

[sebpop]

Ping patch.
Could one of the arm64 maintainers approve this patch and commit?
Thank you.

[sebpop]

Ping patch.

In the Linux kernel rdtsc for Arm64 is implemented with CNTVCT_EL0:

https://lore.kernel.org/patchwork/patch/1305380/

The system register CNTVCT_EL0 can be used to retrieve the counter from
user space. Add rdtsc() for Arm64.

[dmgreen]

Hello. I gave the two registers a look and CNTVCT_EL0 seemed to work better than PMCCNTR_EL0 on the systems I tried. The existing PMCCNTR_EL0 would run into illegal instruction exceptions.

This was added by @ab though and I only tried linux/android. It would be good to hear from them about whether changing this is OK for their platform.

[salvatoredipietro]

For context, MySQL had also to overwrite this function and it is using the CNTVCT_EL0 register for over 3 years now

[ab]

This makes sense, thanks for checking! I'll have a closer look on my side and follow-up if needed

[dmgreen]

OK great, thanks.

If you provide a way to attribute this patch ("name <em@ail>"), I am happy to commit it.

[salvatoredipietro]

In D136999#3982795, @dmgreen wrote:

OK great, thanks.

If you provide a way to attribute this patch ("name <em@ail>"), I am happy to commit it.

sure, "Salvatore Dipietro <[e-mail]>"

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

#undef LCALLNAME5
   setOperationAction(ISD::LOAD, MVT::v32i8, Custom);
   setOperationAction(ISD::LOAD, MVT::v16i16, Custom);
   setOperationAction(ISD::LOAD, MVT::v16f16, Custom);
   setOperationAction(ISD::LOAD, MVT::v8i32, Custom);
   setOperationAction(ISD::LOAD, MVT::v8f32, Custom);
   setOperationAction(ISD::LOAD, MVT::v4f64, Custom);
   setOperationAction(ISD::LOAD, MVT::v4i64, Custom);
 
-  // Lower READCYCLECOUNTER using an mrs from PMCCNTR_EL0.
-  // This requires the Performance Monitors extension.
-  if (Subtarget->hasPerfMon())
-    setOperationAction(ISD::READCYCLECOUNTER, MVT::i64, Legal);
+  // Lower READCYCLECOUNTER using an mrs from CNTVCT_EL0.
 
   if (getLibcallName(RTLIB::SINCOS_STRET_F32) != nullptr &&
       getLibcallName(RTLIB::SINCOS_STRET_F64) != nullptr) {
     // Issue __sincos_stret if available.
     setOperationAction(ISD::FSINCOS, MVT::f64, Custom);
     setOperationAction(ISD::FSINCOS, MVT::f32, Custom);
   } else {
     setOperationAction(ISD::FSINCOS, MVT::f64, Expand);

llvm/lib/Target/AArch64/AArch64InstrInfo.td

def HasCRC           : Predicate<"Subtarget->hasCRC()">,
                                  AssemblerPredicateWithAll<(all_of FeatureCRC), "crc">;
 def HasLSE           : Predicate<"Subtarget->hasLSE()">,
                                  AssemblerPredicateWithAll<(all_of FeatureLSE), "lse">;
 def HasNoLSE         : Predicate<"!Subtarget->hasLSE()">;
 def HasRAS           : Predicate<"Subtarget->hasRAS()">,
                                  AssemblerPredicateWithAll<(all_of FeatureRAS), "ras">;
 def HasRDM           : Predicate<"Subtarget->hasRDM()">,
                                  AssemblerPredicateWithAll<(all_of FeatureRDM), "rdm">;
-def HasPerfMon       : Predicate<"Subtarget->hasPerfMon()">;
 def HasFullFP16      : Predicate<"Subtarget->hasFullFP16()">,
                                  AssemblerPredicateWithAll<(all_of FeatureFullFP16), "fullfp16">;
 def HasFP16FML       : Predicate<"Subtarget->hasFP16FML()">,
                                  AssemblerPredicateWithAll<(all_of FeatureFP16FML), "fp16fml">;
 def HasSPE           : Predicate<"Subtarget->hasSPE()">,
                                  AssemblerPredicateWithAll<(all_of FeatureSPE), "spe">;
 def HasFuseAES       : Predicate<"Subtarget->hasFuseAES()">,
                                  AssemblerPredicateWithAll<(all_of FeatureFuseAES),

 
 let Uses = [ X20 ], Defs = [ X16, X17, LR, NZCV ] in {
 def HWASAN_CHECK_MEMACCESS_SHORTGRANULES : Pseudo<
   (outs), (ins GPR64noip:$ptr, i32imm:$accessinfo),
   [(int_hwasan_check_memaccess_shortgranules X20, GPR64noip:$ptr, (i32 timm:$accessinfo))]>,
   Sched<[]>;
 }
 
-// The cycle counter PMC register is PMCCNTR_EL0.
-let Predicates = [HasPerfMon] in
-def : Pat<(readcyclecounter), (MRS 0xdce8)>;
+// The virtual cycle counter register is CNTVCT_EL0.
+def : Pat<(readcyclecounter), (MRS 0xdf02)>;
 
 // FPCR register
 def : Pat<(i64 (int_aarch64_get_fpcr)), (MRS 0xda20)>;
 def : Pat<(int_aarch64_set_fpcr i64:$val), (MSR 0xda20, GPR64:$val)>;
 
 // Generic system instructions
 def SYSxt  : SystemXtI<0, "sys">;
 def SYSLxt : SystemLXtI<1, "sysl">;

llvm/test/CodeGen/AArch64/mattr-all.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ;; -mattr=+all is not intended to be used for code emitters. Nevertheless,
 ;; llc does not reject it. This test intends to catch behavior changes.
 ; RUN: llc -mtriple=aarch64 -mattr=+all < %s | FileCheck %s
 
 define half @bf16() nounwind {
 ; CHECK-LABEL: bf16:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    adrp x8, .LCPI0_0
 ; CHECK-NEXT:    ldr h0, [x8, :lo12:.LCPI0_0]
 ; CHECK-NEXT:    ret
   ret half 0xH0000
 }
-
-define i64 @perfmon() nounwind {
-; CHECK-LABEL: perfmon:
-; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov x0, xzr
-; CHECK-NEXT:    ret
-  %tmp0 = call i64 @llvm.readcyclecounter()
-  ret i64 %tmp0
-}
-
-declare i64 @llvm.readcyclecounter()

llvm/test/CodeGen/AArch64/readcyclecounter.ll

-; RUN: llc -mtriple=aarch64-unknown-unknown -mattr=+perfmon -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mattr=-perfmon -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=NOPERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a53 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a55 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a510 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a65 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a76 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a77 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a78 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a78c -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-a710 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-r82 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-x1 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=cortex-x2 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=neoverse-e1 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=neoverse-n1 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=neoverse-n2 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
-; RUN: llc -mtriple=aarch64-unknown-unknown -mcpu=neoverse-v1 -asm-verbose=false < %s |\
-; RUN:   FileCheck %s --check-prefix=CHECK --check-prefix=PERFMON
+; RUN: llc -mtriple=aarch64 < %s | FileCheck %s
+
 
 define i64 @test_readcyclecounter() nounwind {
   ; CHECK-LABEL:   test_readcyclecounter:
-  ; PERFMON-NEXT:   mrs x0, PMCCNTR_EL0
-  ; NOPERFMON-NEXT: mov x0, xzr
-  ; CHECK-NEXT:     ret
+  ; CHECK:         // %bb.0:
+  ; CHECK-NEXT:    mrs x0, CNTVCT_EL0
+  ; CHECK-NEXT:    ret
   %tmp0 = call i64 @llvm.readcyclecounter()
   ret i64 %tmp0
 }
 
 declare i64 @llvm.readcyclecounter()