[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

This revision was automatically updated to reflect the committed changes. Closed by commit rG3e0be5610ff0: [MSVC, ARM64] Add __writex18 intrinsics (authored by steplong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126023/new/ https://reviews.llvm

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126023/new/ https://reviews.llvm.org/D126023 ___

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong updated this revision to Diff 431019. steplong added a comment. - Switch to `CharUnits::One()` instead of `getContext().getTypeInChars()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126023/new/ https://reviews.llvm.org/D126023 Files:

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9977 +StoreInst *Store = Builder.CreateAlignedStore( +Val, Ptr, getContext().getTypeAlignInChars(E->getType())); +return Store; I think I'd prefer just "CharUnits::One()",

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong updated this revision to Diff 431013. steplong added a comment. - Changed addrspace(256) to the default addrspace 0 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126023/new/ https://reviews.llvm.org/D126023 Files: clang/include/clang/Ba

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong added a comment. In D126023#3528077 , @rnk wrote: > I was unable to find any documentation for the meaning of AArch64 > addrspace(256), and I wasn't able to figure it out after studying the code in > llvm/lib/Target/AArch64 for ten minutes or s

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

rnk added a comment. I was unable to find any documentation for the meaning of AArch64 addrspace(256), and I wasn't able to figure it out after studying the code in llvm/lib/Target/AArch64 for ten minutes or so. The change seems fine, but please add some documentation as a follow-up. X86 has so

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong updated this revision to Diff 430979. steplong added a comment. The assembly for LLVM now looks like: __writex18byte: strbw1, [x18, w0, uxtw] ret __writex18word: strhw1, [x18, w0, uxtw]

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

efriedma added a comment. `uxtw #3` makes me think you're not generating the right code... the zero-extension hopefully doesn't matter, but the shift is significant. Maybe should be generating `getelementptr i8`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong updated this revision to Diff 430831. steplong added a comment. - Rebased it on top of main instead of D126024 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126023/new/ https://reviews.llvm.org/D126023

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong added reviewers: hans, rnk, efriedma, thakis, mstorsjo. steplong added a comment. The generated assembly is a little different from MSVC's: MSVC LLVM __writex18byte: strbw1, [x18

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

steplong created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. steplong requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. https://docs.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics?view=msvc-170