[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour accepted this revision. bmahjour added a comment. This revision is now accepted and ready to land. Apart from some minor inline comments this revision addresses all my outstanding comments. LGTM. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:17747 +

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

masoud.ataei added a comment. Ready for another round of review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101759/new/ https://reviews.llvm.org/D101759 ___ cfe-commits mailing list

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

masoud.ataei marked 7 inline comments as done. masoud.ataei added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:386 + if (TM.getOptLevel() == CodeGenOpt::Aggressive){ +setOperationAction(ISD::FSIN , MVT::f64, Custom); +

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:386 + if (TM.getOptLevel() == CodeGenOpt::Aggressive){ +setOperationAction(ISD::FSIN , MVT::f64, Custom); +setOperationAction(ISD::FCOS , MVT::f64, Custom); what

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

masoud.ataei added inline comments. Comment at: llvm/include/llvm/Analysis/ScalarFuncs.def:19 +TLI_DEFINE_SCALAR_MASS_FUNC("acosf", "__xl_acosf") +TLI_DEFINE_SCALAR_MASS_FUNC("__acosf_finite", "__xl_acosf") +TLI_DEFINE_SCALAR_MASS_FUNC("acos", "__xl_acos")

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

efriedma added a comment. > I agree. I think the problem is that this patch is trying to decide on a > global lowering strategy for llvm.* math intrinsics in > llvm/lib/Target/PowerPC/PPCISelLowering.cpp but such global decision making > does not go well with finer granularity of fast-math

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour added a comment. In D101759#2971567 , @efriedma wrote: > errno handling for math library functions is a mess. Currently, we don't > model it properly; we just mark the calls "readnone" and hope for the best. > If you don't want to fix that,

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

efriedma added a comment. errno handling for math library functions is a mess. Currently, we don't model it properly; we just mark the calls "readnone" and hope for the best. If you don't want to fix that, just check for readnone for now. I don't think we want to be querying function

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

spatel added a reviewer: efriedma. spatel added a comment. I'm not familiar with this library, and I haven't looked at current state of how we enable/map optional libs in a while... We definitely want to avoid adding another target option/debug flag, and if we can avoid relying on a function

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

lebedev.ri added a reviewer: spatel. lebedev.ri added a comment. In D101759#2967331 , @masoud.ataei wrote: > In D101759#2967250 , @lebedev.ri > wrote: > >> Do we *really* need `-enable-approx-func-fp-math`? >>

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

masoud.ataei marked an inline comment as done. masoud.ataei added a comment. In D101759#2967250 , @lebedev.ri wrote: > Do we *really* need `-enable-approx-func-fp-math`? > I'm pretty sure we are moving away from such global options, onto relying > only

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

lebedev.ri added a comment. Do we *really* need `-enable-approx-func-fp-math`? I'm pretty sure we are moving away from such global options, onto relying only on the per-instruction fast-math flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

masoud.ataei marked 9 inline comments as done. masoud.ataei added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1370 + // to be consistent to PPCGenScalarMASSEntries pass + if (TM.Options.PPCGenScalarMASSEntries && TM.Options.ApproxFuncFPMath) { +

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

masoud.ataei updated this revision to Diff 359385. masoud.ataei added a comment. Removed clang changes from this PR. Removed extra option for MASS pass. Now MASS pass is active with -O3 and approx-func option. Adding another PR for clang changes on approx-func option. Repository: rG LLVM

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

bmahjour added inline comments. Comment at: clang/include/clang/Driver/Options.td:1726 NegFlag>; -def fapprox_func : Flag<["-"], "fapprox-func">, Group, Flags<[CC1Option, NoDriverOption]>, - MarshallingInfoFlag>, ImpliedByAnyOf<[menable_unsafe_fp_math.KeyPath]>; +defm