tingwang added inline comments.
================
Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:192
+ [llvm_float_ty, llvm_float_ty, llvm_float_ty,
llvm_vararg_ty],
+ [IntrNoMem]>;
}
----------------
qiucf wrote:
> Will we support `llvm_f128_ty`?
I'm afraid not at this moment. Document mentions only three types: float,
double, or long double.
================
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);
+ }
----------------
qiucf wrote:
> 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.
Thank you for pointing out this. Verified LowerSELECT_CC is not required here.
This was added due to misconception.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11266
+ case Intrinsic::ppc_maxfe:
+ case Intrinsic::ppc_minfe:
case Intrinsic::ppc_fnmsub:
----------------
qiucf wrote:
> Why only two `fe`?
This is only for ppcf128 during type legalization:
DAGTypeLegalizer::ExpandFloatResult -> CustomLowerNode ->
PPCTargetLowering::ReplaceNodeResults. The other cases seem not hitting here. I
will double check the code path to verify.
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
