[PATCH] D106234: [PowerPC] Fallback to base's implementation of shouldExpandAtomicCmpXchgInIR and shouldExpandAtomicCmpXchgInIR

[Kai Luo via Phabricator via llvm-commits]

lkail added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:17501
     return AtomicExpansionKind::MaskedIntrinsic;
-  return AtomicExpansionKind::None;
+  return TargetLowering::shouldExpandAtomicRMWInIR(AI);
 }
----------------
lkail wrote:
> jsji wrote:
> > With this change, we pretty much fall back on base implementation for *ALL* other types , except for 128 bits. That means quite some change to existing expansion? If so, I think we should at least add tests for all other types and bits to double confirm.
> Before https://reviews.llvm.org/rGb9c3941cd61de1e1b9e4f3311ddfa92394475f4b which override `shouldExpandAtomicRMWInIR`, calling `shouldExpandAtomicRMWInIR` will falls into `TargetLoweringBase`'s
> ```
>   /// Returns how the IR-level AtomicExpand pass should expand the given
>   /// AtomicRMW, if at all. Default is to never expand.
>   virtual AtomicExpansionKind shouldExpandAtomicRMWInIR(AtomicRMWInst *RMW) const {
>     return RMW->isFloatingPointOperation() ?
>       AtomicExpansionKind::CmpXChg : AtomicExpansionKind::None;
>   }
> ```
> After override and this patch, I think all other types remain what they are before https://reviews.llvm.org/rGb9c3941cd61de1e1b9e4f3311ddfa92394475f4b.
And indeed, we're lacking tests while performing `AtomicExpand`, like atomic float operations. I'll add some tests involving `AtomicExpand` as regression tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106234