This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add simplification/combines for llvm.amdgcn.fmul.legacy
ClosedPublic

Authored by foad on Oct 7 2020, 4:13 AM.

Diff Detail

Event Timeline

foad created this revision.Oct 7 2020, 4:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2020, 4:13 AM
foad requested review of this revision.Oct 7 2020, 4:13 AM
arsenm added inline comments.Oct 7 2020, 7:00 AM
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
166

Does the sign matter?

166–176

Do we not have something for this in PatternMatch?

830

Clarify if the sign of zero matters?

844–845

Should check this case first since it's cheaper

844–845

Actually, should we have a more general isKnownFiniteNonZero that happens to check constants?

foad updated this revision to Diff 296714.Oct 7 2020, 9:38 AM

Add some matchers to PatternMatch and use them.

foad marked 4 inline comments as done.Oct 7 2020, 9:46 AM
foad added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
844–845

Perhaps. ValueTracking has isKnownNonZero which doesn't work on floats, but could be made to. It also has CannotBeOrderedLessThanZero.

isKnownFiniteNonZero seems rather specific to me. I realise it's probably more efficient than separate calls to isKnownNeverNaN, isKnownNeverInfinity and isKnownNonZero, but without a lot of refactoring I think there's a serious risk of having dozens of different entrypoints.

As another data point, SelectionDAG has separate isKnownNeverZero and isKnownNeverZeroFloat functions.

Anyway... I decided to duck all of this and just have it work on constants, for now.

foad added a comment.Oct 8 2020, 6:36 AM

It might make sense to rebase this on D89038.

foad updated this revision to Diff 299001.Oct 19 2020, 4:15 AM

Rebase on D89038.

arsenm accepted this revision.Oct 22 2020, 11:54 AM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
844–845

Should add todo to generalize. I was also thinking all the isKnown* functions should return a bitmask, kind of like the inverse of the class intrinsic

This revision is now accepted and ready to land.Oct 22 2020, 11:54 AM
This revision was landed with ongoing or failed builds.Oct 23 2020, 1:31 AM
This revision was automatically updated to reflect the committed changes.
foad marked an inline comment as done.Oct 23 2020, 1:41 AM