[COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

[glandium]

Summary

r344765 added those intrinsics, but used the wrong types.

This affects the release_80 branch.

[smeenai]

+@hans for the release branch.

[efriedma]

Missing testcase changes. (It should be possible to check that we aren't inserting incorrect truncation/extension operations in the IR.)

[TomTan]

Thanks for finding out and fixing this. Seems there is also issue in expanding _WriteStatusReg in CodeGenFunction::EmitAArch64BuiltinExpr. The last argument for _WriteStatusReg is zero extended to __in64, which is not expected (see below link).

https://github.com/llvm-mirror/clang/blob/8070ca12f87e66f76db528c107e9d291f4a91498/lib/CodeGen/CGBuiltin.cpp#L7100

[efriedma]

Yes, we should fix CodeGenFunction::EmitAArch64BuiltinExpr to eliminated the unnecessary calls to CreateZext/CreateTrunc. (With this patch, they're no-ops, but better to clean up the code.)

[glandium]

In D57636#1383566, @efriedma wrote:

(It should be possible to check that we aren't inserting incorrect truncation/extension operations in the IR.)

I don't know how to do that.

[efriedma]

In test/CodeGen/arm64-microsoft-status-reg.cpp, you can write something like // CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2:.*]]), then // CHECK-IR-NEXT: store i64 %[[VAR]] on the next line. See also http://llvm.org/docs/CommandGuide/FileCheck.html .

[glandium]

Updated EmitAArch64BuiltinExpr per https://reviews.llvm.org/D57636#1383751 and the testcase per https://reviews.llvm.org/D57636#1384348

[glandium]

@efriedma can you take another look? Ideally, this should be backported to the release_80 branch, so that would need to be landed asap.

[efriedma]

LGTM. Do you want me to commit this for you?

[glandium]

In D57636#1389971, @efriedma wrote:

LGTM. Do you want me to commit this for you?

Yes, please. Thank you.

cfe/trunk/include/clang/Basic/BuiltinsAArch64.def

 TARGET_HEADER_BUILTIN(_InterlockedDecrement_nf,    "LiLiD*",   "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement_rel,   "LiLiD*",   "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64_acq, "LLiLLiD*", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64_nf,  "LLiLLiD*", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64_rel, "LLiLLiD*", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__getReg, "ULLii", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_ReadStatusReg,  "ii",  "nh", "intrin.h", ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_WriteStatusReg, "vii", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_ReadStatusReg,  "LLii",  "nh", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_WriteStatusReg, "viLLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_AddressOfReturnAddress, "v*", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
 #undef BUILTIN
 #undef LANGBUILTIN
 #undef TARGET_HEADER_BUILTIN

cfe/trunk/lib/CodeGen/CGBuiltin.cpp

llvm::raw_string_ostream(SysRegStr) <<
                        ((SysReg >> 3)  & 15)              << ":" <<
                        ( SysReg        & 7);
 
     llvm::Metadata *Ops[] = { llvm::MDString::get(Context, SysRegStr) };
     llvm::MDNode *RegName = llvm::MDNode::get(Context, Ops);
     llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
 
     llvm::Type *RegisterType = Int64Ty;
-    llvm::Type *ValueType = Int32Ty;
     llvm::Type *Types[] = { RegisterType };
 
     if (BuiltinID == AArch64::BI_ReadStatusReg) {
       llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::read_register, Types);
-      llvm::Value *Call = Builder.CreateCall(F, Metadata);
 
-      return Builder.CreateTrunc(Call, ValueType);
+      return Builder.CreateCall(F, Metadata);
     }
 
     llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::write_register, Types);
     llvm::Value *ArgValue = EmitScalarExpr(E->getArg(1));
-    ArgValue = Builder.CreateZExt(ArgValue, RegisterType);
 
     return Builder.CreateCall(F, { Metadata, ArgValue });
   }
 
   if (BuiltinID == AArch64::BI_AddressOfReturnAddress) {
     llvm::Function *F = CGM.getIntrinsic(Intrinsic::addressofreturnaddress);
     return Builder.CreateCall(F);
   }

cfe/trunk/lib/Headers/intrin.h

 #endif
 
 /*----------------------------------------------------------------------------*\
 |* MS AArch64 specific
 \*----------------------------------------------------------------------------*/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
 long _InterlockedAdd(long volatile *Addend, long Value);
-int _ReadStatusReg(int);
-void _WriteStatusReg(int, int);
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
 
 static inline unsigned short _byteswap_ushort (unsigned short val) {
   return __builtin_bswap16(val);
 }
 static inline unsigned long _byteswap_ulong (unsigned long val) {
   return __builtin_bswap32(val);
 }
 static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {

cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp

 #define ARM64_PMCCNTR_EL0       ARM64_SYSREG(3,3, 9,13,0)  // Cycle Count Register [CP15_PMCCNTR]
 #define ARM64_PMSELR_EL0        ARM64_SYSREG(3,3, 9,12,5)  // Event Counter Selection Register [CP15_PMSELR]
 #define ARM64_PMXEVCNTR_EL0     ARM64_SYSREG(3,3, 9,13,2)  // Event Count Register [CP15_PMXEVCNTR]
 #define ARM64_PMXEVCNTRn_EL0(n) ARM64_SYSREG(3,3,14, 8+((n)/8), (n)%8)    // Direct Event Count Register [n/a]
 #define ARM64_TPIDR_EL0         ARM64_SYSREG(3,3,13, 0,2)  // Thread ID Register, User Read/Write [CP15_TPIDRURW]
 #define ARM64_TPIDRRO_EL0       ARM64_SYSREG(3,3,13, 0,3)  // Thread ID Register, User Read Only [CP15_TPIDRURO]
 #define ARM64_TPIDR_EL1         ARM64_SYSREG(3,0,13, 0,4)  // Thread ID Register, Privileged Only [CP15_TPIDRPRW]
 
-void check_ReadWriteStatusReg(int v) {
-  int ret;
+// From intrin.h
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
+
+void check_ReadWriteStatusReg(__int64 v) {
+  __int64 ret;
   ret = _ReadStatusReg(ARM64_CNTVCT);
-// CHECK-ASM: mrs     x8, CNTVCT_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-ASM: mrs     x0, CNTVCT_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMCCNTR_EL0);
-// CHECK-ASM: mrs     x8, PMCCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-ASM: mrs     x0, PMCCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMSELR_EL0);
-// CHECK-ASM: mrs     x8, PMSELR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+// CHECK-ASM: mrs     x0, PMSELR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTR_EL0);
-// CHECK-ASM: mrs     x8, PMXEVCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+// CHECK-ASM: mrs     x0, PMXEVCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(0));
-// CHECK-ASM: mrs     x8, PMEVCNTR0_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD6:.*]])
+// CHECK-ASM: mrs     x0, PMEVCNTR0_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD6:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(1));
-// CHECK-ASM: mrs     x8, PMEVCNTR1_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD7:.*]])
+// CHECK-ASM: mrs     x0, PMEVCNTR1_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD7:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(30));
-// CHECK-ASM: mrs     x8, PMEVCNTR30_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD8:.*]])
+// CHECK-ASM: mrs     x0, PMEVCNTR30_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD8:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_TPIDR_EL0);
-// CHECK-ASM: mrs     x8, TPIDR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD9:.*]])
+// CHECK-ASM: mrs     x0, TPIDR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD9:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_TPIDRRO_EL0);
-// CHECK-ASM: mrs     x8, TPIDRRO_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD10:.*]])
+// CHECK-ASM: mrs     x0, TPIDRRO_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD10:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_TPIDR_EL1);
-// CHECK-ASM: mrs     x8, TPIDR_EL1
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD11:.*]])
+// CHECK-ASM: mrs     x0, TPIDR_EL1
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD11:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
 
   _WriteStatusReg(ARM64_CNTVCT, v);
-// CHECK-ASM: msr     S3_3_C14_C0_2, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD2:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     S3_3_C14_C0_2, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD2:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_PMCCNTR_EL0, v);
-// CHECK-ASM: msr     PMCCNTR_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD3:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     PMCCNTR_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD3:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_PMSELR_EL0, v);
-// CHECK-ASM: msr     PMSELR_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD4:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     PMSELR_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD4:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_PMXEVCNTR_EL0, v);
-// CHECK-ASM: msr     PMXEVCNTR_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD5:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     PMXEVCNTR_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD5:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(0), v);
-// CHECK-ASM: msr     PMEVCNTR0_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD6:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     PMEVCNTR0_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD6:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(1), v);
-// CHECK-ASM: msr     PMEVCNTR1_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD7:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     PMEVCNTR1_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD7:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(30), v);
-// CHECK-ASM: msr     PMEVCNTR30_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD8:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     PMEVCNTR30_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD8:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_TPIDR_EL0, v);
-// CHECK-ASM: msr     TPIDR_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD9:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     TPIDR_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD9:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_TPIDRRO_EL0, v);
-// CHECK-ASM: msr     TPIDRRO_EL0, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD10:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     TPIDRRO_EL0, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD10:.*]], i64 %[[VAR]])
 
   _WriteStatusReg(ARM64_TPIDR_EL1, v);
-// CHECK-ASM: msr     TPIDR_EL1, x8
-// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD11:.*]], i64 {{%.*}})
+// CHECK-ASM: msr     TPIDR_EL1, x0
+// CHECK-IR: %[[VAR:.*]] = load i64,
+// CHECK-IR-NEXT: call void @llvm.write_register.i64(metadata ![[MD11:.*]], i64 %[[VAR]])
 }
 
 // CHECK-IR: ![[MD2]] = !{!"3:3:14:0:2"}
 // CHECK-IR: ![[MD3]] = !{!"3:3:9:13:0"}
 // CHECK-IR: ![[MD4]] = !{!"3:3:9:12:5"}
 // CHECK-IR: ![[MD5]] = !{!"3:3:9:13:2"}
 // CHECK-IR: ![[MD6]] = !{!"3:3:14:8:0"}
 // CHECK-IR: ![[MD7]] = !{!"3:3:14:8:1"}
 // CHECK-IR: ![[MD8]] = !{!"3:3:14:11:6"}
 // CHECK-IR: ![[MD9]] = !{!"3:3:13:0:2"}
 // CHECK-IR: ![[MD10]] = !{!"3:3:13:0:3"}
 // CHECK-IR: ![[MD11]] = !{!"3:0:13:0:4"}