This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Or combine to BFI function
ClosedPublic

Authored by samparker on Oct 17 2017, 6:20 AM.

Details

Summary

Extract the functionality to combine OR to BFI into its own function.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker created this revision.Oct 17 2017, 6:20 AM
samparker updated this revision to Diff 119326.Oct 17 2017, 8:24 AM

Inserted a couple of missed lines to get the node operands, dag and VT.

john.brawn added inline comments.Oct 18 2017, 3:40 AM
lib/Target/ARM/ARMISelLowering.cpp
10402 ↗(On Diff #119326)

You're now returning N in an SDValue and that gets returned from PerformORCombine, so now we will be adding the new nodes to the DAG combiner worklist. Is this intentional? Either way something needs to be adjusted here (and in the other places where this comment occurs).

10557–10559 ↗(On Diff #119326)

It would be better to do this in PerformORCombineToBFI (in general less preconditions is better).

samparker added inline comments.Oct 18 2017, 3:51 AM
lib/Target/ARM/ARMISelLowering.cpp
10402 ↗(On Diff #119326)

Yes, returning an SDValue of the original SDNode tells the combiner than N is now dead. I will update the comment.

10557–10559 ↗(On Diff #119326)

Ah yes, cheers.

samparker updated this revision to Diff 119455.Oct 18 2017, 3:58 AM

Updated some comments.

This revision is now accepted and ready to land.Oct 24 2017, 8:25 AM
Closed by commit rL316563: [ARM] OrCombineToBFI function (authored by sam_parker). · Explain WhyOct 25 2017, 1:38 AM
This revision was automatically updated to reflect the committed changes.