[PATCH] D130191: [RISCV] Teach ComputeNumSignBitsForTargetNode about masked atomic intrinsics

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
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.
----------------
reames wrote:
> 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);
> 
> 
> 
Hi Philip, I've drafted a description that is very similar to yours, but made a few small alterations:
* Referencing cmpxchg as well at atomicrmw
* unaligned => narrow.

I think this is close enough to your suggestion that I'm going to go ahead and commit, but obviously just shout if you think that didn't match your intent and we can resolve in a follow-up patch.

Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130191/new/

https://reviews.llvm.org/D130191