[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

This revision was automatically updated to reflect the committed changes. Closed by commit rL353493: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg (authored by efriedma, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

glandium added a comment. In D57636#1389971 , @efriedma wrote: > LGTM. Do you want me to commit this for you? Yes, please. Thank you. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57636/new/

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM. Do you want me to commit this for you? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57636/new/ https://reviews.llvm.org/D57636

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

glandium added a comment. @efriedma can you take another look? Ideally, this should be backported to the release_80 branch, so that would need to be landed asap. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57636/new/ https://reviews.llvm.org/D57636

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

glandium updated this revision to Diff 185218. glandium edited the summary of this revision. glandium added a comment. Updated EmitAArch64BuiltinExpr per https://reviews.llvm.org/D57636#1383751 and the testcase per https://reviews.llvm.org/D57636#1384348 Repository: rC Clang CHANGES SINCE

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

efriedma added a comment. 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

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

glandium added a comment. 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. Repository: rC Clang CHANGES SINCE LAST

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

efriedma added a comment. 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.) Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

TomTan added a comment. 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).

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

efriedma added a subscriber: TomTan. efriedma added a comment. Missing testcase changes. (It should be possible to check that we aren't inserting incorrect truncation/extension operations in the IR.) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57636/new/

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

smeenai added subscribers: hans, smeenai. smeenai added reviewers: rnk, efriedma. smeenai added a comment. +@hans for the release branch. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57636/new/ https://reviews.llvm.org/D57636

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

glandium created this revision. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar. Herald added a project: clang. r344765 added those intrinsics, but used the wrong types. I /think/ the code in EmitAArch64BuiltinExpr does the right thing, though, but I'm not familiar with it.