This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Implement widenScalar for saturating add/sub
ClosedPublic

Authored by arsenm on Jul 13 2020, 5:21 AM.

Details

Summary

Add a placeholder legality rule for AMDGPU until the rest of the
actions are handled.

Diff Detail

Event Timeline

arsenm created this revision.Jul 13 2020, 5:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2020, 5:21 AM
foad accepted this revision.Jul 13 2020, 6:32 AM

Looks fine but can be simplified.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1644–1645

This isn't needed if you make the changes below.

1660–1663

These can both be anyext.

1671

There's no need to make this conditional on IsSigned if we're just about to truncate anyway.

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
1431

"selectively"

This revision is now accepted and ready to land.Jul 13 2020, 6:32 AM
arsenm marked an inline comment as done.Jul 13 2020, 6:36 AM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1671

How else would I know whether to prefer ashr or lshr?

foad added inline comments.Jul 13 2020, 6:37 AM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1671

Why would it make any difference? Can't you just pick one arbitrarily?

arsenm marked an inline comment as done.Jul 13 2020, 6:46 AM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1671

First I don't know whether the target has a reason to prefer one over the other. I also would assume using the right shift to preserve the expected number of sign bits for the signed result would be more useful for downstream simplifications once the trunc is folded out

foad added inline comments.Jul 13 2020, 6:50 AM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1671

OK, I won't object if you want to leave it like that. The other comment about anyext still stands.