This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Augment addsat/subsat lowering with an optional type
Needs ReviewPublic

Authored by arsenm on Jul 16 2020, 9:58 AM.

Details

Summary

Extend and perform the lowering in the wider type if provided. This
provides a sort of lower with combined widen scalar action.
widenScalar will always widen and produce the original opcode in a
wider type, whereas this will use different opcodes.

This ends up saving an instruction in the uaddsat case. If you know
you are widening, the add in the wider result can't overflow so only
the clamp of the maximum value is needed. In other cases, it can avoid
introducing more instructions that need further widening, saving
avoiding intermediate legalization artifacts.

Use this to handle the sub-32 bit cases for AMDGPU when the clamp
modifier isn't available. This brings the results closer to
SelectionDAG, which duplicates nearly identical expansions of these in
both LegalizeIntegerTypes and LegalizeDAG.

Diff Detail

Event Timeline

arsenm created this revision.Jul 16 2020, 9:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2020, 9:58 AM
foad added a comment.Aug 6 2020, 4:02 AM

Looks OK technically. As an alternative, would it be possible to widen, and then have the lowering spot that the operands are both extended from a smaller type, so they're known to have a small range, so we can use the more efficient lowering? That would avoid the need for this new "optional type" concept.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
5883–5884

Could use getLowBitsSet and getOneBitSet, but I suppose it would be annoyingly asymmetrical.

5883–5884

Surely we should take advantage of DoPromote to simplify this? You can do a normal add/sub and then clamp the result to MinVal and MaxVal.

5899–5900

Use APInt::getLowBitsSet?

Looks OK technically. As an alternative, would it be possible to widen, and then have the lowering spot that the operands are both extended from a smaller type, so they're known to have a small range, so we can use the more efficient lowering? That would avoid the need for this new "optional type" concept.

I tried this, but it complicates the heuristic for which lowering strategy to use. If we need to promote, the isLegalOrCustom check for min/max won't work for the narrower type

arsenm updated this revision to Diff 287500.Aug 24 2020, 2:34 PM

Handle promotion signed case. Avoiding the explicit hint is trickier