[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 13 02:57:04 PDT 2021


nemanjai added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9754
   "this builtin is only valid on POWER7 or later CPUs">;
+def err_ppc_builtin_only_on_pwr9 : Error<
+  "this builtin is only valid on POWER9 or later CPUs">;
----------------
No longer required after https://reviews.llvm.org/D105501


================
Comment at: clang/lib/Sema/SemaChecking.cpp:3356
+  case PPC::BI__builtin_ppc_maddld:
+    return SemaFeatureCheck(*this, TheCall, "power9-vector",
+                            diag::err_ppc_builtin_only_on_pwr9);
----------------
NeHuang wrote:
> amyk wrote:
> > This is just a question. 
> > Is `power9-vector` the correct feature check in these cases? Does it matter if these are not vector instructions?
> yeah, we planned using this feature to do the sema check for `pwr9` only (or later cpus) builtins.
Now that https://reviews.llvm.org/D105501 is approved, please change this to
`isa-v30-instructions` please.


================
Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:1532
+      : GCCBuiltin<"__builtin_ppc_cmprb">,
+        Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
+  def int_ppc_setb
----------------
Shouldn't operand 0 be marked as an immediate here?


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.td:5263
+let Predicates = [IsISA3_0] in {
+def : Pat<(i32 (int_ppc_cmprb i32:$a, i32:$b, i32:$c)),
+          (i32 (SETB (CMPRB imm:$a, $b, $c)))>;
----------------
Shouldn't `$a` be a `u1imm`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875



More information about the llvm-commits mailing list