[PATCH] D158673: [SDAG][RISCV] Avoid neg instructions when lowering atomic_load_sub with a constant rhs

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

In D158673#4615347 <https://reviews.llvm.org/D158673#4615347>, @craig.topper wrote:

> In D158673#4615235 <https://reviews.llvm.org/D158673#4615235>, @jrtc27 wrote:
>
>> In D158673#4615052 <https://reviews.llvm.org/D158673#4615052>, @craig.topper wrote:
>>
>>> 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.
>>
>> I don't think you need that part, just check if add is legal, use that if so, otherwise fall back on a libcall?
>
> AArch64 seems to create `add` libcalls from `sub`. But maybe that isn't intentional?

For outlined atomics? That's intentional, they correspond to the available LSE instructions, and behave as if they were instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158673