Differential D126023
[MSVC, ARM64] Add __writex18 intrinsics Authored by steplong on May 19 2022, 3:09 PM.
Details https://docs.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics?view=msvc-170 void __writex18byte(unsigned long, unsigned char) void __writex18word(unsigned long, unsigned short) void __writex18dword(unsigned long, unsigned long) void __writex18qword(unsigned long, unsigned __int64) Given the lack of documentation of the intrinsics, we chose to align the offset with just
Diff Detail
Event TimelineComment Actions The generated assembly is a little different from MSVC's: MSVC LLVM
__writex18byte:
strb w1, [x18, x0] strb w1, [x18, w0, uxtw]
__writex18word:
strh w1, [x18, x0] strh w1, [x18, w0, uxtw #1]
__writex18dword:
str w1, [x18, x0] str w1, [x18, w0, uxtw #2]
__writex18qword:
str x1, [x18, x0] str x1, [x18, w0, uxtw #3]Comment Actions 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? Comment Actions The assembly for LLVM now looks like: __writex18byte:
strb w1, [x18, w0, uxtw]
ret
__writex18word:
strh w1, [x18, w0, uxtw]
ret
__writex18dword:
str w1, [x18, w0, uxtw]
ret
__writex18qword:
str x1, [x18, w0, uxtw]
retComment Actions 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 some of its address spaces documented here, not that this is the best place: Comment Actions Hmm yea I don't think addrspace(256) is correct. I was using D31248 as a reference and addrspace(256) makes sense for that patch.
| ||||||
I think I'd prefer just "CharUnits::One()", rather than getTypeAlignInChars(); given the lack of documentation, it's not clear if the offset is required to be properly aligned.