This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] add store (load float*) pattern to isProfitableToHoist
ClosedPublic

Authored by shchenz on Jun 22 2020, 7:03 PM.

Details

Summary

store (load float*) can be optimized to store(load i32*) in InstCombine pass.

Add store (load float*) to isProfitableToHoist to make sure we don't break the opt in InstCombine pass.

Diff Detail

Event Timeline

shchenz created this revision.Jun 22 2020, 7:03 PM
shchenz edited the summary of this revision. (Show Details)Jun 22 2020, 10:12 PM

gentle ping

gentle ping...

jsji added inline comments.Jul 17 2020, 8:21 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15997

Looks like the FIXME is wrong? The default is true, so we should be adding more patterns which are NOT profitable to hoist instead?

16027

!I->hasOneUse() is common for all, can we move it up before switch?
Also the Type check can be done after Store opcode check?

16030

If these conditions are *copied* from combineLoadToOperationType, please also copy comments.
Otherwise please add comments about why we need these condition, especially isSwiftError?

Also do we have any IR test with SwiftError?

16033

This is also common, can be moved before switch.

llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-load-i32.ll
1

Precommit the new testcase first so that we can see the diff here.

shchenz updated this revision to Diff 278975.Jul 18 2020, 12:33 AM
shchenz marked 6 inline comments as done.

address @jsji comments:
1: code refactor
2: split out NFC patch for new added testcase

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

SwiftError attribute can only be applied to pointer to pointer type, so I->getType()->getTypeID() != Type::FloatTyID should get rid of this case.

jsji accepted this revision as: jsji.Jul 19 2020, 7:52 PM

LGTM.

This revision is now accepted and ready to land.Jul 19 2020, 7:52 PM
This revision was automatically updated to reflect the committed changes.