[PATCH] D130191: [RISCV] Teach ComputeNumSignBitsForTargetNode about Intrinsic::riscv_masked_cmpxchg_i64

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 21 10:08:23 PDT 2022


asb added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9682
+      // ANDing the result with a mask. Therefore, the sign bits are the
+      // minimum of 33 (from the LR_W) and the number of sign bits in the mask
+      // operand.
----------------
reames wrote:
> I don't follow this comment.  Is there some documentation you can point to for what this intrinsic does?  Or a good pointer in code to understand it?
> 
> If I'm gathering this correctly, the and-by-mask is to handle the zext of the type less than XLEN?  Not following where the sign bit below that is coming from.  
The intrinsics are underdocumented - I'll loop back round and fix that. For general context on our atomics lowering approach, see [here](https://lists.llvm.org/pipermail/llvm-dev/2018-June/123993.html).

As for this specific code comment: it may be because it's the end of the day, but I started trying to better explain and confused myself. So it may be I have a mistake, the code comment needs improving, or both. Just leaving this comment now, as it may be it's best for you to hold off on taking a proper look until I've had a chance to re-review.


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

https://reviews.llvm.org/D130191



More information about the llvm-commits mailing list