Page MenuHomePhabricator

[PowerPC] add IR level isFMAFasterThanFMulAndFAdd - NFC
ClosedPublic

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

Details

Summary

NFC patch for add IR level target hook isFMAFasterThanFMulAndFAdd

Diff Detail

Event Timeline

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

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

shchenz added a comment.EditedTue, Mar 17, 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.EditedTue, Mar 17, 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.Tue, Mar 17, 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.Tue, Mar 17, 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.Wed, Mar 18, 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.Wed, Mar 18, 12:32 AM

LGTM now.

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