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);
+ setOperationAction(ISD::FCOS , MVT::f64, Custom);
----------------
bmahjour wrote:
> what about tan, acos, and the others?
These are the list of math functions that llvm creates intrinsic call for them.
There is no llvm intrinsic for tan, acos and other math functions which (exist
in MASS and) are not in this list.
================
Comment at: llvm/test/CodeGen/PowerPC/lower-intrinsics-afn-mass.ll:148
+
+attributes #1 = { "approx-func-fp-math"="true" }
----------------
bmahjour wrote:
> All the calls have `afn`....why do we need this attribute?
Removed
================
Comment at: llvm/test/CodeGen/PowerPC/lower-intrinsics-fast-mass.ll:148
+
+attributes #1 = { "no-infs-fp-math"="true" "no-nans-fp-math"="true"
"no-signed-zeros-fp-math"="true" "approx-func-fp-math"="true" }
----------------
bmahjour wrote:
> do we need this attribute? Can we remove it or have separate tests for
> functions with attributes?
Removed
================
Comment at: llvm/test/CodeGen/PowerPC/lower-intrinsics-mass-aix.ll:1
+; RUN: llc -O3 -mtriple=powerpc-ibm-aix-xcoff < %s | FileCheck %s
+
----------------
bmahjour wrote:
> We don't really need a separate aix file. Can we just add a run line with the
> aix triple to `llvm/test/CodeGen/PowerPC/lower-intrinsics-nofast-mass.ll`?
Done
================
Comment at: llvm/test/CodeGen/PowerPC/lower-scalar-mass-fast.ll:796
+; Without nnan ninf afn nsz flags on the call instruction
+define float @acosf_f32_nofast(float %a) {
+; CHECK-LABEL: acosf_f32_nofast
----------------
bmahjour wrote:
> shouldn't the tests starting from here move to a different file? This test
> file is called ...mass-fast.ll so one would expect it only contains tests
> with fast-math flag on.
Done
================
Comment at:
llvm/test/CodeGen/PowerPC/pow-025-075-intrinsic-scalar-mass-fast.ll:246
+; CHECK-LNX-NEXT: lfs 2, .LCPI4_0@toc@l(3)
+; CHECK-LNX-NEXT: bl __xl_powf_finite
+; CHECK-LNX-NEXT: nop
----------------
bmahjour wrote:
> How come pow -> sqrt conversion didn't happen here?
Honestly, I am not sure why the conversion is not happening in this case. But
without this patch we will get `powf` call (the conversion is not happening
again). So this is a separate issue that someone needs to look at independent
of this patch.
================
Comment at:
llvm/test/CodeGen/PowerPC/pow-025-075-nointrinsic-scalar-mass-fast.ll:22
+; CHECK-LNX-NEXT: lfs 2, .LCPI0_0@toc@l(3)
+; CHECK-LNX-NEXT: bl __xl_powf_finite
+; CHECK-LNX-NEXT: nop
----------------
bmahjour wrote:
> so pow->sqrt translation never happens for non-intrinsic `pow`. Is that
> expected? If so, are we planning to recognize these patterns inside
> PPCGenScalarMASSEntries in the future and do the translation as part of that
> transform?
Correct, pow->sqrt translation is not happening for none intrinsic cases. It is
the case independent of this patch. I guess the reason is DAGCombiner only
apply this optimization on llvm intrinsics. This is an issue that either we
need to handle it in DAGCombiner (same as intrinsic one) or in MASS pass. I
feel DAGCombiner is a better option and I think this is also a separate issue.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101759/new/
https://reviews.llvm.org/D101759
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
