[PATCH] D158673: [SDAG][RISCV] Avoid neg instructions when lowering atomic_load_sub with a constant rhs
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 24 13:50:41 PDT 2023
craig.topper added a comment.
In D158673#4614879 <https://reviews.llvm.org/D158673#4614879>, @jrtc27 wrote:
> In D158673#4614877 <https://reviews.llvm.org/D158673#4614877>, @dtcxzyw wrote:
>
>> In D158673#4614846 <https://reviews.llvm.org/D158673#4614846>, @jrtc27 wrote:
>>
>>> Can we not just teach SelectionDAG to handle ATOMIC_LOAD_SUB=Expand, ATOMIC_LOAD_ADD=Legal?
>>
>> I think we can let `RISCVTargetLowering::shouldExpandAtomicRMWInIR(atomicrmw sub)` return `Expand` and handle it in `RISCVTargetLowering::emitExpandAtomicRMW`.
>
> That still makes it target-dependent, but the expansion at the IR or SelectionDAG level is target-independent. There is no reason we should have separate code for RISCV and AArch64. Sharing code rather than duplicating functionality is good practice when it makes sense, and I don't see a reason why it wouldn't here.
We can probably do this in LegalizeDAG::ExpandNode. First we need to change very target that really wants a sub LibCall to pass LibCall instead of Expand to setOperationAction. Then we could add an Expand action for ATOMIC_SUB to ExpandNode that uses NEG+ATOMIC_ADD. RISC-V and AArch64 could use the Expand action. Need to change AArch64TargetLowering constructor to figure out the cases to use Expand.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158673/new/
https://reviews.llvm.org/D158673
More information about the llvm-commits
mailing list