[PATCH] D41856: [PowerPC] Zero-extend the compare operand for ATOMIC_CMP_SWAP
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 10 11:48:27 PST 2018
efriedma added a comment.
> Do you mean that the general approach of updating the nodes during custom legalization is fragile
Yes. The problem is that you're essentially attaching additional semantics to a target-independent opcode (specifically, that the "unused" bits of the ATOMIC_CMP_SWAP must be zero). DAGCombine doesn't understand this, and can therefore break your assumptions. This has come up before in other contexts (e.g. https://reviews.llvm.org/D31331).
The clean approach is to introduce a target-specific node (e.g. PPCISD::ATOMIC_CMP_SWAP), and custom-lower ISD::ATOMIC_CMP_SWAP to PPCISD::ATOMIC_CMP_SWAP; that way, it's clear your node has different semantics.
================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:8876
+ // ATOMIC_CMP_SWAP is Legal and ATOMIC_CMP_SWAP_WITH_SUCCESS needs to be
+ // Expanded.
+ return ToExpand ? SDValue() : Op;
----------------
Do you really need to custom-lower ATOMIC_CMP_SWAP_WITH_SUCCESS, given that we're just going to call into the custom lowering code again with the generated ATOMIC_CMP_SWAP?
Repository:
rL LLVM
https://reviews.llvm.org/D41856
More information about the llvm-commits
mailing list