t.p.northover created this revision.
Herald added subscribers: danielkiss, jdoerfert, asbirlea, hiraditya,
kristof.beyls, mcrosier.
Herald added projects: clang, LLVM.
Some of the system registers readable on AArch64 & ARM platforms return
different values with each read (for example a timer counter), these shouldn't
be hoisted outside loops or otherwise interfered with, but the normal
@llvm.read_register intrinsic is only considered to read memory.
This introduces a separate @llvm.read_volatile_register intrinsic and maps all
system-registers on ARM platforms to use it for the __builtin_arm_rsr calls.
Registers declared with `asm("r9")` or similar are unaffected.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80911
Files:
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-arm.c
clang/test/CodeGen/builtins-arm64.c
llvm/docs/LangRef.rst
llvm/include/llvm/IR/Intrinsics.td
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/test/Transforms/LICM/read-volatile-register.ll
Index: llvm/test/Transforms/LICM/read-volatile-register.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LICM/read-volatile-register.ll
@@ -0,0 +1,30 @@
+; RUN: opt -S -licm %s | FileCheck %s
+
+; Volatile register shouldn't be hoisted ourside loops.
+define i32 @test_read() {
+; CHECK-LABEL: define i32 @test_read()
+; CHECK: br label %loop
+; CHECK: loop:
+; CHECK: %counter = tail call i64 @llvm.read_volatile_register
+
+entry:
+ br label %loop
+
+loop:
+ %i = phi i32 [ 0, %entry ], [ %i.next, %inc ]
+ %counter = tail call i64 @llvm.read_volatile_register.i64(metadata !1)
+ %tst = icmp ult i64 %counter, 1000
+ br i1 %tst, label %inc, label %done
+
+inc:
+ %i.next = add nuw nsw i32 %i, 1
+ br label %loop
+
+done:
+ ret i32 %i
+}
+
+declare i64 @llvm.read_register.i64(metadata)
+declare i64 @llvm.read_volatile_register.i64(metadata)
+
+!1 = !{!"cntpct_el0"}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5665,6 +5665,7 @@
TLI.getFrameIndexTy(DAG.getDataLayout()),
getValue(I.getArgOperand(0))));
return;
+ case Intrinsic::read_volatile_register:
case Intrinsic::read_register: {
Value *Reg = I.getArgOperand(0);
SDValue Chain = getRoot();
Index: llvm/include/llvm/IR/Intrinsics.td
===================================================================
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -456,6 +456,9 @@
[IntrReadMem], "llvm.read_register">;
def int_write_register : Intrinsic<[], [llvm_metadata_ty, llvm_anyint_ty],
[], "llvm.write_register">;
+def int_read_volatile_register : Intrinsic<[llvm_anyint_ty], [llvm_metadata_ty],
+ [IntrHasSideEffects],
+ "llvm.read_volatile_register">;
// Gets the address of the local variable area. This is typically a copy of the
// stack, frame, or base pointer depending on the type of prologue.
Index: llvm/docs/LangRef.rst
===================================================================
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -11538,9 +11538,11 @@
'``llvm.localrecover``'.
.. _int_read_register:
+.. _int_read_volatile_register:
.. _int_write_register:
-'``llvm.read_register``' and '``llvm.write_register``' Intrinsics
+'``llvm.read_register``', '``llvm.read_volatile_register``', and
+'``llvm.write_register``' Intrinsics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Syntax:
@@ -11550,6 +11552,8 @@
declare i32 @llvm.read_register.i32(metadata)
declare i64 @llvm.read_register.i64(metadata)
+ declare i32 @llvm.read_volatile_register.i32(metadata)
+ declare i64 @llvm.read_volatile_register.i64(metadata)
declare void @llvm.write_register.i32(metadata, i32 @value)
declare void @llvm.write_register.i64(metadata, i64 @value)
!0 = !{!"sp\00"}
@@ -11557,17 +11561,21 @@
Overview:
"""""""""
-The '``llvm.read_register``' and '``llvm.write_register``' intrinsics
-provides access to the named register. The register must be valid on
-the architecture being compiled to. The type needs to be compatible
-with the register being read.
+The '``llvm.read_register``', '``llvm.read_volatile_register``', and
+'``llvm.write_register``' intrinsics provide access to the named register.
+The register must be valid on the architecture being compiled to. The type
+needs to be compatible with the register being read.
Semantics:
""""""""""
-The '``llvm.read_register``' intrinsic returns the current value of the
-register, where possible. The '``llvm.write_register``' intrinsic sets
-the current value of the register, where possible.
+The '``llvm.read_register``' and '``llvm.read_volatile_register``' intrinsics
+return the current value of the register, where possible. The
+'``llvm.write_register``' intrinsic sets the current value of the register,
+where possible.
+
+A call to '``llvm.read_volatile_register``' is assumed to have side-effects
+and possibly return a different value each time (e.g. for a timer register).
This is useful to implement named register global variables that need
to always be mapped to a specific register, as is common practice on
Index: clang/test/CodeGen/builtins-arm64.c
===================================================================
--- clang/test/CodeGen/builtins-arm64.c
+++ clang/test/CodeGen/builtins-arm64.c
@@ -68,7 +68,7 @@
__typeof__(__builtin_arm_rsr("1:2:3:4:5")) rsr(void);
uint32_t rsr() {
- // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+ // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_volatile_register.i64(metadata ![[M0:[0-9]]])
// CHECK-NEXT: trunc i64 [[V0]] to i32
return __builtin_arm_rsr("1:2:3:4:5");
}
@@ -76,12 +76,12 @@
__typeof__(__builtin_arm_rsr64("1:2:3:4:5")) rsr64(void);
uint64_t rsr64(void) {
- // CHECK: call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+ // CHECK: call i64 @llvm.read_volatile_register.i64(metadata ![[M0:[0-9]]])
return __builtin_arm_rsr64("1:2:3:4:5");
}
void *rsrp() {
- // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+ // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_volatile_register.i64(metadata ![[M0:[0-9]]])
// CHECK-NEXT: inttoptr i64 [[V0]] to i8*
return __builtin_arm_rsrp("1:2:3:4:5");
}
Index: clang/test/CodeGen/builtins-arm.c
===================================================================
--- clang/test/CodeGen/builtins-arm.c
+++ clang/test/CodeGen/builtins-arm.c
@@ -222,19 +222,19 @@
}
unsigned rsr() {
- // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.read_register.i32(metadata ![[M0:.*]])
+ // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.read_volatile_register.i32(metadata ![[M0:.*]])
// CHECK-NEXT: ret i32 [[V0]]
return __builtin_arm_rsr("cp1:2:c3:c4:5");
}
unsigned long long rsr64() {
- // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_register.i64(metadata ![[M1:.*]])
+ // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_volatile_register.i64(metadata ![[M1:.*]])
// CHECK-NEXT: ret i64 [[V0]]
return __builtin_arm_rsr64("cp1:2:c3");
}
void *rsrp() {
- // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.read_register.i32(metadata ![[M2:.*]])
+ // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.read_volatile_register.i32(metadata ![[M2:.*]])
// CHECK-NEXT: [[V1:[%A-Za-z0-9.]+]] = inttoptr i32 [[V0]] to i8*
// CHECK-NEXT: ret i8* [[V1]]
return __builtin_arm_rsrp("sysreg");
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -6226,6 +6226,12 @@
llvm::ConstantInt::get(Int32Ty, Value));
}
+enum SpecialRegisterAccessKind {
+ NormalRead,
+ VolatileRead,
+ Write,
+};
+
// Generates the IR for the read/write special register builtin,
// ValueType is the type of the value that is to be written or read,
// RegisterType is the type of the register being written to or read from.
@@ -6233,7 +6239,7 @@
const CallExpr *E,
llvm::Type *RegisterType,
llvm::Type *ValueType,
- bool IsRead,
+ SpecialRegisterAccessKind AccessKind,
StringRef SysReg = "") {
// write and register intrinsics only support 32 and 64 bit operations.
assert((RegisterType->isIntegerTy(32) || RegisterType->isIntegerTy(64))
@@ -6258,8 +6264,12 @@
assert(!(RegisterType->isIntegerTy(32) && ValueType->isIntegerTy(64))
&& "Can't fit 64-bit value in 32-bit register");
- if (IsRead) {
- llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::read_register, Types);
+ if (AccessKind != Write) {
+ assert(AccesKind == NormalRead || AccessKind == VolatileRead);
+ llvm::Function *F = CGM.getIntrinsic(
+ AccessKind == VolatileRead ? llvm::Intrinsic::read_volatile_register
+ : llvm::Intrinsic::read_register,
+ Types);
llvm::Value *Call = Builder.CreateCall(F, Metadata);
if (MixedTypes)
@@ -6631,9 +6641,11 @@
BuiltinID == ARM::BI__builtin_arm_wsr64 ||
BuiltinID == ARM::BI__builtin_arm_wsrp) {
- bool IsRead = BuiltinID == ARM::BI__builtin_arm_rsr ||
- BuiltinID == ARM::BI__builtin_arm_rsr64 ||
- BuiltinID == ARM::BI__builtin_arm_rsrp;
+ SpecialRegisterAccessKind AccessKind = Write;
+ if (BuiltinID == ARM::BI__builtin_arm_rsr ||
+ BuiltinID == ARM::BI__builtin_arm_rsr64 ||
+ BuiltinID == ARM::BI__builtin_arm_rsrp)
+ AccessKind = VolatileRead;
bool IsPointerBuiltin = BuiltinID == ARM::BI__builtin_arm_rsrp ||
BuiltinID == ARM::BI__builtin_arm_wsrp;
@@ -6652,7 +6664,8 @@
ValueType = RegisterType = Int32Ty;
}
- return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType, IsRead);
+ return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType,
+ AccessKind);
}
// Deal with MVE builtins
@@ -8560,9 +8573,11 @@
BuiltinID == AArch64::BI__builtin_arm_wsr64 ||
BuiltinID == AArch64::BI__builtin_arm_wsrp) {
- bool IsRead = BuiltinID == AArch64::BI__builtin_arm_rsr ||
- BuiltinID == AArch64::BI__builtin_arm_rsr64 ||
- BuiltinID == AArch64::BI__builtin_arm_rsrp;
+ SpecialRegisterAccessKind AccessKind = Write;
+ if (BuiltinID == AArch64::BI__builtin_arm_rsr ||
+ BuiltinID == AArch64::BI__builtin_arm_rsr64 ||
+ BuiltinID == AArch64::BI__builtin_arm_rsrp)
+ AccessKind = VolatileRead;
bool IsPointerBuiltin = BuiltinID == AArch64::BI__builtin_arm_rsrp ||
BuiltinID == AArch64::BI__builtin_arm_wsrp;
@@ -8580,7 +8595,8 @@
ValueType = Int32Ty;
}
- return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType, IsRead);
+ return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType,
+ AccessKind);
}
if (BuiltinID == AArch64::BI_ReadStatusReg ||
@@ -14409,7 +14425,7 @@
}
case AMDGPU::BI__builtin_amdgcn_read_exec: {
CallInst *CI = cast<CallInst>(
- EmitSpecialRegisterBuiltin(*this, E, Int64Ty, Int64Ty, true, "exec"));
+ EmitSpecialRegisterBuiltin(*this, E, Int64Ty, Int64Ty, NormalRead, "exec"));
CI->setConvergent();
return CI;
}
@@ -14418,7 +14434,7 @@
StringRef RegName = BuiltinID == AMDGPU::BI__builtin_amdgcn_read_exec_lo ?
"exec_lo" : "exec_hi";
CallInst *CI = cast<CallInst>(
- EmitSpecialRegisterBuiltin(*this, E, Int32Ty, Int32Ty, true, RegName));
+ EmitSpecialRegisterBuiltin(*this, E, Int32Ty, Int32Ty, NormalRead, RegName));
CI->setConvergent();
return CI;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
