Page MenuHomePhabricator

[PowerPC] Scalar IBM MASS library conversion pass
Needs ReviewPublic

Authored by masoud.ataei on May 3 2021, 7:46 AM.

Details

Reviewers
etiotto
pjeeva01
renenkel
bmahjour
Group Reviewers
Restricted Project
Summary

This patch introduces an option to enable conversions from math function calls
to MASS library calls. To resolves calls generated with these conversions, one
need to link libxlopt.a library.

This patch is tested on PowerPC Linux and AIX.

Diff Detail

Unit TestsFailed

TimeTest
200 msx64 debian > LLVM.Examples/OrcV2Examples::lljit-with-remote-debugging.test
Script: -- : 'RUN: at line 4'; /mnt/disks/ssd0/agent/llvm-project/build/bin/LLJITWithRemoteDebugging /mnt/disks/ssd0/agent/llvm-project/llvm/test/Examples/OrcV2Examples/Inputs/argc_sub1_elf.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --check-prefix=CHECK1 /mnt/disks/ssd0/agent/llvm-project/llvm/test/Examples/OrcV2Examples/lljit-with-remote-debugging.test

Event Timeline

masoud.ataei created this revision.May 3 2021, 7:46 AM
masoud.ataei requested review of this revision.May 3 2021, 7:46 AM
steven.zhang added a reviewer: Restricted Project.May 17 2021, 10:24 PM
bmahjour added inline comments.May 18 2021, 4:28 PM
llvm/include/llvm/Analysis/ScalarFuncs.def
17

shouldn't these map from llvm.* intrinsics to mass entry points as well?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
1361

why are these being handled here instead of PPCGenScalarMASSEntries.cpp?

masoud.ataei added inline comments.May 19 2021, 1:07 PM
llvm/include/llvm/Analysis/ScalarFuncs.def
17

llvm intrinsics is handled in PPCISelLowering.cpp.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
1361

We are not handling llvm intrinsics in PPCGenScalarMASSEntries.cpp because we don't want to block any type of existing optimizations (like pow(x,0.5) --> sqrt(x)) and future optimizations (like https://reviews.llvm.org/D94543 ?).

bmahjour added inline comments.Tue, May 25, 12:42 PM
llvm/lib/Target/PowerPC/PPCGenScalarMASSEntries.cpp
70

There should be a todo comment to handle non-finite entries using fewer fast-math flags.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
1361

I see, could you please put a comment in the code to explain that? Alternatively you can put the comment at the top of llvm/include/llvm/Analysis/ScalarFuncs.def.

1361

Instead of TM.Options.UnsafeFPMath we should test for the individual fast-math flags that are required for safety. Checking for "unsafe-fp-math" has a few drawbacks:

  1. To make clang enable that flag it is necessary but not enough to specify -funsafe-math-optimizations! You'd have to specify -fno-math-errno as well.
  2. Clang sets the "unsafe-fp-math" flag when all four of -fno-math-errno -fassociative-math -freciprocal-math -fno-signed-zeros are specified, regardless of other flags... For example this command does the conversion to the _finite calls despite the user request to honor NaNs. clang t.c -c -O3 -fno-math-errno -fassociative-math -freciprocal-math -fno-signed-zeros -fhonor-nans

Even if the clang inconsistencies/issues are resolved, it would still be better to check for the individual flags for finer control and for consistency with other front-ends.

llvm/test/CodeGen/PowerPC/lower-intrinsics-mass-aix.ll
1

why not just use the default CHECK prefix? CHECK-ALL and CHECK-LWR don't distinguish anything based on this run command.

19

CHECK-DFLT is not in the list of prefixes defined.

llvm/test/CodeGen/PowerPC/lower-intrinsics-nofast-mass.ll
147

Remove this line, #1 is unused.