[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