128 bit shifts constants expanded when shift parts is legal

I have a target where 128-bit shifts generate 2 instructions. I match shift_parts in the td file. This works if the count is in a register but not if the count is a constant. It appears the code in DAGTypeLegalizer::ExpandIntRes_Shift() always expands constant shift counts into 4 instructions BEFORE checking if shift_parts is legal. This seems wrong.

The 4-instruction sequence you mention is easy to pattern-match. If your target supports ISD::FSHL or ISD::FSHR, DAGCombine will pattern-match the code generated by the legalizer to those ops. (SHL_PARTS mainly exists to support variable shifts, since they’re harder to pattern-match after legalization.)

So if I support funnel shifts this magic will just happen. Do I still have to support shift_PARTS?

If your target has funnel shifts, just pattern-matching the output of ExpandShiftWithUnknownAmountBit probably produces something reasonable. (It outputs something like funnel-shift+regular shift+select+select.) If that’s not optimal, then you probably need shift_PARTS.

Any targets do as you suggest such that I might study :slight_smile:

For which part? x86 is a reasonable example of a target that uses funnel shifts extensively. For pattern-matching a constant shift on a target without a native funnel shift, see tryCombineToEXTR in AArch64ISelLowering.cpp.

Thank you.