Global Isel:
Does not lower FSHR if the amount of shifts is a constant.
Lowers FSHL to FSHR instead of EXTR.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
GlobalISel:
Does not lower FSHR if the amount of shifts is a constant.
Lowers FSHL to FSHR instead of EXTR.
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
870 | I think that the s64, s32 combo will never come up. The s32, x64 only comes up due to the way this is being legalized. |
You lower at least s37 and s128 without any attempts to legalize them. There may be some opportunities, at least for s37.
I would expect that i128 is more important than i37. It may be a job for combining, as opposed to legalization (or better generic code for legalizing fsh's)
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
72 | number of shifts -> shift amount | |
84 | It might be worth making this an APInt, so that it doesn't need to convert again below. | |
90 | These have more () brackets than they need. |
Amount is created as an APInt instead of converting it later on
Replaced number of shifts in comments with shift amount
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
1025–1053 | Can you explain what you mean? My understanding was that returning true meant "this has been legalized", so the G_FSHR with a constant shift between 0 and BitWidth is legal. (So it ends up reusing the tablegen patterns for fshr). Non-constant shifts are illegal (expanded) and fshl are transformed to a fshr. | |
llvm/test/CodeGen/AArch64/funnel-shift.ll | ||
474 | This one may be incorrect I think. It should be returning the other operand from the test below (ideally without a extr). |
llvm/include/llvm/Target/GlobalISel/Combine.td | ||
---|---|---|
387 ↗ | (On Diff #547705) | Can you split this out into a separate patch (and move it down next to funnel_shift_to_rotate, maybe with a name that includes funnel_shift). It could do with a mir test to show the combine works in isolation. It's probably best to keep the optimization separate from the legalization though. |
Can you add the llvm/test/CodeGen/AArch64/funnel-shift.ll test changes back to this patch too? Otherwise this looks good to me.
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
1020 | Maybe: // Lower non-constant shifts and leave zero shifts to the optimizer. |
number of shifts -> shift amount