Author: Tim Northover Date: 2020-07-15T09:47:36+01:00 New Revision: 5165b2b5fd5fd62c5a34970be81c79231844804c
URL: https://github.com/llvm/llvm-project/commit/5165b2b5fd5fd62c5a34970be81c79231844804c DIFF: https://github.com/llvm/llvm-project/commit/5165b2b5fd5fd62c5a34970be81c79231844804c.diff LOG: AArch64+ARM: make LLVM consider system registers volatile. Some of the system registers readable on AArch64 and 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. Added: llvm/test/Transforms/LICM/read-volatile-register.ll Modified: 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/GlobalISel/IRTranslator.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 35a93a7889f4..34f4f21746f7 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -6361,6 +6361,12 @@ Value *CodeGenFunction::GetValueForARMHint(unsigned BuiltinID) { 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. @@ -6368,7 +6374,7 @@ static Value *EmitSpecialRegisterBuiltin(CodeGenFunction &CGF, 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)) @@ -6393,8 +6399,12 @@ static Value *EmitSpecialRegisterBuiltin(CodeGenFunction &CGF, 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) @@ -6773,9 +6783,11 @@ Value *CodeGenFunction::EmitARMBuiltinExpr(unsigned BuiltinID, 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; @@ -6794,7 +6806,8 @@ Value *CodeGenFunction::EmitARMBuiltinExpr(unsigned BuiltinID, ValueType = RegisterType = Int32Ty; } - return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType, IsRead); + return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType, + AccessKind); } // Deal with MVE builtins @@ -8834,9 +8847,11 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID, 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; @@ -8854,7 +8869,8 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID, ValueType = Int32Ty; } - return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType, IsRead); + return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType, + AccessKind); } if (BuiltinID == AArch64::BI_ReadStatusReg || @@ -14797,7 +14813,7 @@ Value *CodeGenFunction::EmitAMDGPUBuiltinExpr(unsigned BuiltinID, } 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; } @@ -14806,7 +14822,7 @@ Value *CodeGenFunction::EmitAMDGPUBuiltinExpr(unsigned BuiltinID, 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; } diff --git a/clang/test/CodeGen/builtins-arm.c b/clang/test/CodeGen/builtins-arm.c index f3c4ecaeee90..98e4621971b7 100644 --- a/clang/test/CodeGen/builtins-arm.c +++ b/clang/test/CodeGen/builtins-arm.c @@ -222,19 +222,19 @@ uint64_t mrrc2() { } 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"); diff --git a/clang/test/CodeGen/builtins-arm64.c b/clang/test/CodeGen/builtins-arm64.c index f5cf997e5226..35dbb09ea7ff 100644 --- a/clang/test/CodeGen/builtins-arm64.c +++ b/clang/test/CodeGen/builtins-arm64.c @@ -68,7 +68,7 @@ int32_t jcvt(double v) { __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 @@ uint32_t rsr() { __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"); } diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 8bb808a3256c..9b58b9dfb171 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -11643,9 +11643,11 @@ the escaped allocas are allocated, which would break attempts to use '``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: @@ -11655,6 +11657,8 @@ Syntax: 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"} @@ -11662,17 +11666,21 @@ Syntax: 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 diff erent 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 diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td index 9b14a07eb7b9..4918ea876df6 100644 --- a/llvm/include/llvm/IR/Intrinsics.td +++ b/llvm/include/llvm/IR/Intrinsics.td @@ -458,6 +458,9 @@ def int_read_register : Intrinsic<[llvm_anyint_ty], [llvm_metadata_ty], [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. diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp index bbdefe3e5ca4..8f6643b2f193 100644 --- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp +++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp @@ -1598,6 +1598,7 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID, case Intrinsic::sideeffect: // Discard annotate attributes, assumptions, and artificial side-effects. return true; + case Intrinsic::read_volatile_register: case Intrinsic::read_register: { Value *Arg = CI.getArgOperand(0); MIRBuilder diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index c8b72abb9b7d..1d596c89c911 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5698,6 +5698,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, 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(); diff --git a/llvm/test/Transforms/LICM/read-volatile-register.ll b/llvm/test/Transforms/LICM/read-volatile-register.ll new file mode 100644 index 000000000000..1836d3762b3b --- /dev/null +++ b/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"} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
