[llvm] [DAGCombiner] Move handling of atomic loads from SystemZ to DAGCombiner (NFC). (PR #86484)

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 26 10:49:40 PDT 2024


================
@@ -1454,6 +1454,28 @@ class TargetLoweringBase {
            getLoadExtAction(ExtType, ValVT, MemVT) == Custom;
   }
 
+  /// Same as getLoadExtAction, but for atomic loads.
+  LegalizeAction getAtomicLoadExtAction(unsigned ExtType, EVT ValVT,
----------------
JonPsson1 wrote:

Interesting question. 

getAtomicLoadExtAction() is used to check for a specific width / extension type before performing the DAG combine on a load, while getExtendForAtomicOps() is used as a general preference check without any concern of the specific VT, including other things than loads.

During expansion of ATOMIC_CMP_SWAP_WITH_SUCCESS, an ATOMIC_CMP_SWAP is emitted, which may become a libcall it says. Then getExtendForAtomicOps() is used to handle that result, so that is indeed a very general, broad target characteristic, even including a call result.

I guess it looks to me to be more simple to let targets that only have one of these available simply state that by overriding getExtendForAtomicOps(). It will help them everywhere - libcalls, computeKnownBits, ComputeNumSignBits, ...  I guess such a target have either the real instruction or a pseudo implemented to do the extension, for all VTs.

It may be possible to have a target instead of doing that, specify all the available legal atomic load extensions. From these, it could then be deduced that e.g. only SIGN_EXTEND is available and a general assumption could be made. But that's not perfect either, so wouldn't be worth the effort, probably...

It *should* be possible to unify these one way or the other, but it doesn't seem simple to me.


https://github.com/llvm/llvm-project/pull/86484


More information about the llvm-commits mailing list