[PATCH] D106678: [PowerPC] Add pwr7 and pwr10 support to IBM MASSV pass on AIX

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCLowerMASSVEntries.cpp:74
 StringRef PPCLowerMASSVEntries::getCPUSuffix(const PPCSubtarget *Subtarget) {
-  // Assume Power8 when Subtarget is unavailable.
+  // Assume generic when Subtarget is unavailable.
   if (!Subtarget)
----------------
Do these generic entries actually exist in MASS (ie can link correctly)?


================
Comment at: llvm/lib/Target/PowerPC/PPCLowerMASSVEntries.cpp:77
+    return "";
+  if (Subtarget->isAIXABI() && Subtarget->hasP10Vector())
+    return "_P10";
----------------
why not just check for `Subtarget->hasP10Vector()`? 
I understand P10 work on LoP is in progress, but not sure about the state of that work. This patch causes an observable change in behaviour where we used to resolve to P8 and link successfully on P10, but now we abort with the fatal error below. If P10 entries do currently exist on LoP P10 and they are functional (perhaps not with best performance), then we should just let it link with P10 entries. Otherwise we should add a comment and link to P8 entries like before.


================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/massv-altivec.ll:16
 ; CHECK-LABEL: @cbrt_f64(
-; CHECK-NOT: __cbrtd2_P8{{.*}}<2 x double>
 ; CHECK: ret void
----------------
I'm wondering if it is possible to get these _P8 calls in the IR in any other way (ie does clang generate them to implement any of the builtins)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106678