[PATCH] D130191: [RISCV] Teach ComputeNumSignBitsForTargetNode about masked atomic intrinsics
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 2 13:08:30 PDT 2022
reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.
LGTM w/requested changes.
As an optional follow up, I find myself wondering if the 64 bit versions should actually have a 64 bit return type. Having them instead have i32 operands and return types - to model the emulated access actually being done - and then explicitly sign extended afterwards would get the same effect here with less code and less confusion.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9899
+ case Intrinsic::riscv_masked_cmpxchg_i64:
+ // The return value of these intrinsics is produced by an LR_W or
+ // atomicrmw_W instruction and so is sign-extended from 32 bits.
----------------
Ok, I finally wrapped my head around all of the machinery here, and this is correct. However, I want to suggest some changes.
First, the comment:
// riscv_masked_atomicrmw_* represents an emulated unaligned atomicrmw operation at the minimum supported atomicrmw width whose result is then sign extended to XLEN. With +A, the minimum width is 32 for both 64 and 32.
Second, please add the asserts...
assert(XLenVT == 64)
assert(getMinCmpXchgSizeInBits() == 32);
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130191/new/
https://reviews.llvm.org/D130191
More information about the llvm-commits
mailing list