qiucf added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9899
+def err_ppc_unsupported_argument_type : Error<
+ "unsupported argument type %0 for target %1">;
def err_x86_builtin_invalid_rounding : Error<
----------------
Can we re-use `err_typecheck_convert_incompatible`?
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16290-16310
+ case PPC::BI__builtin_ppc_maxfe:
+ case PPC::BI__builtin_ppc_maxfl:
+ case PPC::BI__builtin_ppc_maxfs:
+ case PPC::BI__builtin_ppc_minfe:
+ case PPC::BI__builtin_ppc_minfl:
+ case PPC::BI__builtin_ppc_minfs: {
+ if (BuiltinID == PPC::BI__builtin_ppc_maxfe)
----------------
================
Comment at: clang/lib/Sema/SemaChecking.cpp:3913
+ case PPC::BI__builtin_ppc_minfs: {
+ // FIXME: remove below check once -mlong-double-128 is supported on AIX.
+ if (Context.getTargetInfo().getTriple().isOSAIX() &&
----------------
I think we don't need this fixme.
================
Comment at: clang/test/CodeGen/PowerPC/builtins-ppc.c:65
+ // CHECK: call double (double, double, double, ...) @llvm.ppc.maxfl(double
%0,
+ // double %1, double %2, double %3)
+ res = __builtin_ppc_maxfl(a, b, c, d);
----------------
Don't break CHECK lines.
================
Comment at: clang/test/Sema/builtins-ppc.c:13-17
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown -DTEST_MAXMIN
-fsyntax-only \
+// RUN: -verify %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -DTEST_MAXMINFE_AIX
-fsyntax-only \
+// RUN: -verify %s
----------------
================
Comment at: clang/test/Sema/builtins-ppc.c:63-83
+#ifdef TEST_MAXMIN
+void test_maxmin() {
+ long double fe;
+ double fl;
+ float fs;
+ __builtin_ppc_maxfe(fl, fl, fl, fl); // expected-error-re {{requires
argument of {{.*}} type (passed in {{.*}})}}
+ __builtin_ppc_minfe(fl, fl, fl, fl); // expected-error-re {{requires
argument of {{.*}} type (passed in {{.*}})}}
----------------
I don't know if it's convention in tests, but looks simpler.
================
Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:192
+ [llvm_float_ty, llvm_float_ty, llvm_float_ty,
llvm_vararg_ty],
+ [IntrNoMem]>;
}
----------------
Will we support `llvm_f128_ty`?
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10583-10587
+ for (unsigned i = 4, e = Op.getNumOperands(); i < e; ++i) {
+ if (Op.getOperand(i).getValueType() != Op.getValueType())
+ report_fatal_error("Intrinsic::ppc_[max|min]f[e|l|s] must have uniform
"
+ "type arguments");
+ }
----------------
We can make it even simpler.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10594
+ // Below selection order follows XLC behavior: start from the last but one
+ // operand, move towards the first operand, end with the last operand.
+ unsigned I, Cnt;
----------------
We don't need to mention old behavior.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10595-10596
+ // operand, move towards the first operand, end with the last operand.
+ unsigned I, Cnt;
+ I = Cnt = Op.getNumOperands() - 2;
+ SDValue Res = Op.getOperand(I);
----------------
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10598-10602
+ for (--I; Cnt != 0; --Cnt, I = (--I == 0 ? (Op.getNumOperands() - 1) : I))
{
+ Res = LowerSELECT_CC(
+ DAG.getSelectCC(dl, Res, Op.getOperand(I), Res, Op.getOperand(I),
CC),
+ DAG);
+ }
----------------
I don't think we need to manually call `LowerSELECT_CC` here. SelectionDAG
knows `ppc_fp128` should not be custom lowered.
This also makes the case pass. Thus D122462 is not needed.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11266
+ case Intrinsic::ppc_maxfe:
+ case Intrinsic::ppc_minfe:
case Intrinsic::ppc_fnmsub:
----------------
Why only two `fe`?
================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-maxmin.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-linux < %s | FileCheck %s
+
----------------
Can we add `pwr8` run line?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122478/new/
https://reviews.llvm.org/D122478
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
