[PATCH] D117006: [PowerPC] Add custom lowering for SELECT_CC fp128 using xsmaxcqp

Ting Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 17 20:28:31 PST 2022


tingwang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1345
+    if (Subtarget.hasP10Vector()) {
+      setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);
+    }
----------------
shchenz wrote:
> tingwang wrote:
> > qiucf wrote:
> > > We already have a `setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);` under `Subtarget.hasVSX()`, so don't need this.
> > That setOperationAction is located under the else part of Subtarget.hasP9Vector(), since P9Vector is part of P9InheritableFeatures, P10 test Subtarget.hasP9Vector() as true, so not hit that setOperationAction. Since this is supported by P10, and do not see Subtarget.hasP10Vector() predicate in this file, so added one under the Subtarget.hasAltivec() big block.
> What if we set `Custom` for `SELECT_CC` for all targets `hasVSX()`? I don't see a reason why we can not set ` setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);` on Power9. If so, we can just set `SELECT_CC` for type `f128` under `hasVSX()` instead of setting it two times for P10 and hasVSX()?
Had a try doing Custom on Power9 to SELECT_CC, that path caused dump. Following is the path taken by fp128/Power9/SELECT_CC, and picks up PPC::XSCMPUQP:
PPCDAGToDAGISel::Select
-> case ISD::SELECT_CC
-->   SelectCC()

Seems the expected code paths for Power9 and Power10 are different, then we still need a predicate to differentiate that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117006



More information about the llvm-commits mailing list