Page MenuHomePhabricator

[DAG] MatchRotate - support rotate-by-constant of illegal types
ClosedPublic

Authored by RKSimon on Thu, Nov 4, 8:09 AM.

Details

Summary

This is a WIP patch to fix some of the regressions in D77804.

By folding to rotate/funnel-shift by constant amounts for illegal types, we prevent SimplifyDemandedBits from destroying the patterns prematurely, allowing us to use the rotate/funnel-shift legalization that was added in D112443.

Diff Detail

Event Timeline

RKSimon created this revision.Thu, Nov 4, 8:09 AM
RKSimon requested review of this revision.Thu, Nov 4, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Nov 4, 8:09 AM
Herald added a subscriber: MaskRay. · View Herald Transcript
RKSimon added inline comments.Thu, Nov 11, 4:19 AM
llvm/test/CodeGen/X86/vector-rotate-128.ll
2334

This seems to be the last regression - we fold (and (shift x, c1), c2) but fail to do the same for (and (rotate x, c1), c2)

RKSimon added inline comments.Sat, Nov 13, 8:50 AM
llvm/test/CodeGen/X86/vector-rotate-128.ll
2334

This can be better handled with improved VPTERNLOG generation: D113827

RKSimon updated this revision to Diff 387074.Sun, Nov 14, 6:10 AM
RKSimon retitled this revision from [DAG] MatchRotate - support rotate-by-constant of illegal types (WIP) to [DAG] MatchRotate - support rotate-by-constant of illegal types.
RKSimon edited the summary of this revision. (Show Details)

Rebase - ready for review

I think i can't spot any more regressions.
Looks good to me, but one more review would be best.

llvm/test/CodeGen/X86/vector-rotate-128.ll
2332–2333

So we exchanged this or + and-by-folded-load for constant pool load + vpternlogq ?

RKSimon added inline comments.Sun, Nov 14, 7:35 AM
llvm/test/CodeGen/X86/vector-rotate-128.ll
2332–2333

Yes, that was what we went with for D113827 as a base implementation - D113845 will improve broadcast folds for ternlog

RKSimon added inline comments.Mon, Nov 15, 8:33 AM
llvm/test/CodeGen/RISCV/rv64zbp.ll
1863

@craig.topper Is this change OK? The test is called grev16_32 and neither target generates a grev op.

craig.topper added inline comments.Mon, Nov 15, 11:48 AM
llvm/test/CodeGen/RISCV/rv64zbp.ll
1863

I think it is ok. It's now matches grev16_i32_fshl and grev16_i32_fshr which are the same as this using intrinsics instead of shifts and or.

Our assumption was that a rotate would be at least as good as a grev instruction.

lebedev.ri accepted this revision.Fri, Nov 19, 1:50 AM

LG, anyone else?

This revision is now accepted and ready to land.Fri, Nov 19, 1:50 AM