Do a sign-extension in a compare-and-swap of 32 bit in RV64A

[fpallares]

Summary

AtomicCmpSwapWithSuccess is legalised into an AtomicCmpSwap plus a comparison. This requires an extension of the value which, by default, is a zero-extension.

When we later lower AtomicCmpSwap into a PseudoCmpXchg32 and then expanded in RISCVExpandPseudoInsts.cpp, the lr.w instruction does a sign-extension.

This mismatch of extensions causes the comparison to fail when the compared value is negative.

This change overrides TargetLowering::getExtendForAtomicOps for RISC-V so it does a sign-extension instead.

[fpallares]

Ping.

[asb]

Great catch - thank you!

Could you please modify the test to use update_llc_test_checks.py (which on balance we find easier to maintain), and add a comment indicating the original motivation for the test.

[fpallares]

Thank you. I've updated the test using 'update_llc_test_checks.py' and I've added a comment as suggested.

[asb]

Looks good to me. Thanks again for catching this issue, and of course for submitting the fix!

[fpallares]

I'm glad it helps, thank you for your time!

llvm/trunk/lib/Target/RISCV/RISCVISelLowering.h

public:
   bool shouldInsertFencesForAtomic(const Instruction *I) const override {
     return isa<LoadInst>(I) || isa<StoreInst>(I);
   }
   Instruction *emitLeadingFence(IRBuilder<> &Builder, Instruction *Inst,
                                 AtomicOrdering Ord) const override;
   Instruction *emitTrailingFence(IRBuilder<> &Builder, Instruction *Inst,
                                  AtomicOrdering Ord) const override;
 
+  ISD::NodeType getExtendForAtomicOps() const override {
+    return ISD::SIGN_EXTEND;
+  }
+
 private:
   void analyzeInputArgs(MachineFunction &MF, CCState &CCInfo,
                         const SmallVectorImpl<ISD::InputArg> &Ins,
                         bool IsRet) const;
   void analyzeOutputArgs(MachineFunction &MF, CCState &CCInfo,
                          const SmallVectorImpl<ISD::OutputArg> &Outs,
                          bool IsRet, CallLoweringInfo *CLI) const;
   // Lower incoming arguments, copy physregs into vregs

llvm/trunk/test/CodeGen/RISCV/atomic-cmpxchg-flag.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64IA %s
+
+; This test ensures that the output of the 'lr.w' instruction is sign-extended.
+; Previously, the default zero-extension was being used and 'cmp' parameter
+; higher bits were masked to zero for the comparison.
+
+define i1 @cmpxchg_i32_seq_cst_seq_cst(i32* %ptr, i32 signext %cmp,
+        i32 signext %val) {
+; RV64IA-LABEL: cmpxchg_i32_seq_cst_seq_cst:
+; RV64IA:       # %bb.0: # %entry
+; RV64IA-NEXT:  .LBB0_1: # %entry
+; RV64IA-NEXT:    # =>This Inner Loop Header: Depth=1
+; RV64IA-NEXT:    lr.w.aqrl a3, (a0)
+; RV64IA-NEXT:    bne a3, a1, .LBB0_3
+; RV64IA-NEXT:  # %bb.2: # %entry
+; RV64IA-NEXT:    # in Loop: Header=BB0_1 Depth=1
+; RV64IA-NEXT:    sc.w.aqrl a4, a2, (a0)
+; RV64IA-NEXT:    bnez a4, .LBB0_1
+; RV64IA-NEXT:  .LBB0_3: # %entry
+; RV64IA-NEXT:    xor a0, a3, a1
+; RV64IA-NEXT:    seqz a0, a0
+; RV64IA-NEXT:    ret
+entry:
+  %0 = cmpxchg i32* %ptr, i32 %cmp, i32 %val seq_cst seq_cst
+  %1 = extractvalue { i32, i1 } %0, 1
+  ret i1 %1
+}