This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Disable CTR Loop generate for fma with the PPC double double type.
ClosedPublic

Authored by amyk on Aug 11 2021, 10:16 AM.

Details

Summary

It is possible to generate the llvm.fmuladd.ppcf128 intrinsic, and there is no actual
FMA instruction that corresponds to this intrinsic call for ppcf128. Thus, this
intrinsic needs to remain as a call as it cannot be lowered to any instruction, which
also means we need to disable CTR loop generation for fma involving the ppcf128 type.
This patch accomplishes this behaviour.

Diff Detail

Event Timeline

amyk created this revision.Aug 11 2021, 10:16 AM
amyk requested review of this revision.Aug 11 2021, 10:16 AM
amyk updated this revision to Diff 365850.Aug 11 2021, 2:14 PM

Address clang-format comment for PPCTargetTransformInfo.cpp.

shchenz added inline comments.Aug 11 2021, 6:51 PM
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
462

Enable/disable CTR loop generation can not make sure the Intrinsic::fmuladd be kept as a call. Can we move these lines to line 500?

llvm/test/CodeGen/PowerPC/disable-ctr-ppcf128.ll
150

Can we narrow down the case a little bit? maybe one call is enough?

amyk updated this revision to Diff 366133.Aug 12 2021, 3:35 PM

Address review comments:

  • Move case the fmuladd lower in mightUseCTR.
  • Shorten test case to one llvm.fmuladd.ppcf128 intrinsic call.
amyk marked 2 inline comments as done.Aug 12 2021, 3:35 PM
shchenz accepted this revision.Aug 12 2021, 10:37 PM

Look good to me except for some minor comments for the test cases part.

llvm/test/CodeGen/PowerPC/disable-ctr-ppcf128.ll
16

Is it necessary to test we don't generate a CTR loop?

21

none of these args are used in the function body, so I think they can be removed and the type definition for %0, %1, %2, %3 can also be removed?

21

We can also removed the unnecessary function attributes like linkonce_odr hidden local_unnamed_addr and local_unnamed_addr?

This revision is now accepted and ready to land.Aug 12 2021, 10:37 PM
amyk updated this revision to Diff 366193.Aug 12 2021, 10:54 PM

Clean up test case a little more - removing unnecessary function attributes/arguments.

amyk marked 3 inline comments as done.Aug 12 2021, 10:56 PM

@shchenz I've cleaned up the test case a little bit more by removing the attributes and arguments (that I realized we were not utilizing). Thanks for catching these details! :-)