This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Use FSHR in DAGTypeLegalizer::ExpandIntRes_MULFIX
ClosedPublic

Authored by bjope on Aug 31 2019, 3:23 AM.

Details

Summary

Simplify the right shift of the intermediate result (given
in four parts) by using funnel shift.

There are some impact on lit tests, but that seems to be
related to register allocation differences due to how FSHR
is expanded on X86 (giving a slightly different operand order
for the explicit OR operations compared to the old code).

Diff Detail

Repository
rL LLVM

Event Timeline

bjope created this revision.Aug 31 2019, 3:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2019, 3:23 AM

This is just an NFC cleanup right?

bjope added a comment.Aug 31 2019, 6:12 AM

This is just an NFC cleanup right?

Yes, basically (if it still can be called NFC when it could impact codegen like this).
The old SRL/SHL/OR patterns would be combined to funnel shift (when FSHR is custom/legal). Now we select FSHR directly, and instead it will be expanded to an SRL/SHL/OR pattern when FSHR isn't legal.
So the result would be the same, except that the expand of FSHR puts the operands in opposite order in the ISD::OR node.

I also assume that there could be odd cases when the old SRL/SHL/OR pattern would be combined with some other operations before matching it as an FSHR. So it might be hard to guarantee that the DAG is identical after type legalization + DAG combine.

RKSimon accepted this revision.Sep 2 2019, 11:10 AM

LGTM - cheers.

This revision is now accepted and ready to land.Sep 2 2019, 11:10 AM
This revision was automatically updated to reflect the committed changes.