This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] add IR level isFMAFasterThanFMulAndFAdd - NFC
ClosedPublic

Authored by shchenz on Mar 16 2020, 8:14 PM.

Details

Summary

NFC patch for add IR level target hook isFMAFasterThanFMulAndFAdd

Diff Detail

Event Timeline

shchenz created this revision.Mar 16 2020, 8:14 PM

I don't think this is a NFC patch as you are adding hook for IR version.

shchenz added a comment.EditedMar 17 2020, 9:21 PM

I don't think this is a NFC patch as you are adding hook for IR version.

The IR hook does not have any caller like NFC patches only adding a helper function without any caller?

steven.zhang added a comment.EditedMar 17 2020, 10:23 PM

I don't think this is a NFC patch as you are adding hook for IR version.

The IR hook does not have any caller like NFC patches only adding a helper function without any caller?

Well, that's really confusing to have hook without caller. And yes, it is NFC.

steven.zhang added inline comments.Mar 17 2020, 10:58 PM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15370–15371

Don't get the scalar type as the parameter of "Ty" in IR version is the type of the function.

15370–15371

You don't need this check any more as you don't call the getSimpleVT now.

I don't think this is a NFC patch as you are adding hook for IR version.

The IR hook does not have any caller like NFC patches only adding a helper function without any caller?

Well, that's really confusing to have hook without caller. And yes, it is NFC.

actually there is a user isProfitableToHoist in AArch64 target for same purpose as what I do for this patch.

shchenz updated this revision to Diff 251002.Mar 17 2020, 11:51 PM
shchenz marked 2 inline comments as done.

Thanks for your comments @steven.zhang . Updated accordingly.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15370–15371

Right.

15370–15371

I think You mean in IR version, I already call getScalarType() , so no need to call it again?

steven.zhang added inline comments.Mar 18 2020, 12:31 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15370–15371

My understanding is that, these two hooks have completely the same semantics basing on different data structure. We should pass through the type instead of strip the vector type. And it doesn't make sense as the decision is made inside IR version hook, we cannot make any assumption that it didn't care about the vector type.

steven.zhang accepted this revision.Mar 18 2020, 12:32 AM

LGTM now.

This revision is now accepted and ready to land.Mar 18 2020, 12:32 AM
This revision was automatically updated to reflect the committed changes.