This is an archive of the discontinued LLVM Phabricator instance.

PowerPC-specific builtin constrained FP enablement
ClosedPublic

Authored by ajwock on Jun 17 2020, 8:55 AM.

Details

Summary

This change enables PowerPC compiler builtins to generate constrained floating point operations when clang is indicated to do so.

A couple of possibly unexpected backend divergences between constrained floating point and regular behavior are highlighted under the test tag FIXME-CHECK. This may be something for those on the PPC backend to look at.

Diff Detail

Event Timeline

ajwock created this revision.Jun 17 2020, 8:55 AM
jsji added reviewers: qiucf, Restricted Project.Jun 17 2020, 10:47 AM
steven.zhang added inline comments.Jun 17 2020, 6:28 PM
clang/lib/CodeGen/CGBuiltin.cpp
14270

Can we do it like this to avoid the duplicate if statement ?

if (...)
   ID = Builder.getIsFPConstrained() ? Intrinsic::experimental_constrained_floor : Intrinsic::floor;
 ...
 return Builder.getIsFPConstrained() ? Builder. CreateConstrainedFPCall() : Builder.CreateCall();
ajwock updated this revision to Diff 272186.Jun 19 2020, 2:36 PM

Took steven.zhang's suggestion, added REQUIRES line to diff. Hopefully addressed harbormaster concerns.

LGTM overall with some minor comments.

clang/lib/CodeGen/CGBuiltin.cpp
14238

line too long. Please format the code with clang-format.

clang/test/CodeGen/builtins-ppc-fpconstrained.c
4

Do we have to specify option "-disable-O0-optnone -Wall -Wno-unused -Werror" ?

ajwock updated this revision to Diff 272330.Jun 21 2020, 9:36 PM

It seems one of the issues that my tests revealed was already remedied in very recent changes, causing my test to fail. I changed the test to reflect that while also taking steven's recommendations.

ajwock marked 3 inline comments as done.Jun 21 2020, 9:36 PM
steven.zhang accepted this revision.Jun 22 2020, 3:04 AM

LGTM now and thank you for doing this. Please hold on for 2-3 days in case others have comments. And thank you for pointing a potential issue of the folding of fneg + fma. We will take a look later.

This revision is now accepted and ready to land.Jun 22 2020, 3:04 AM
This revision was automatically updated to reflect the committed changes.