This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Do not fcanonicalize const splat padded with undef
ClosedPublic

Authored by Petar.Avramovic on Jun 16 2021, 11:50 AM.

Details

Summary

Recognize constant splat padded with undef in isCanonicalized.
Fcanonicalize will be removed by RemoveFcanonicalize in post-legalizer
combiner. We will treat undef as value that will result in a splat
in clamp combine after regbankselect.

Diff Detail

Event Timeline

Petar.Avramovic requested review of this revision.Jun 16 2021, 11:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2021, 11:50 AM
arsenm requested changes to this revision.Jun 16 2021, 11:52 AM

This is incorrect. Undef could be anything, so it could be a nan

This revision now requires changes to proceed.Jun 16 2021, 11:52 AM

This is required to match splat that was padded with undef into clamp D90052. When undef is used to pad with element that does not affect the result we interpret it in a way to clamp other elements.

This is required to match splat that was padded with undef into clamp D90052. When undef is used to pad with element that does not affect the result we interpret it in a way to clamp other elements.

This at least needs a parameter to ignore undefs. It's not really true to the name of the function otherwise

foad added a comment.Jun 17 2021, 1:52 AM

matchUnaryPredicate already has an AllowUndefs argument, so you could do the same.

Incidentally ValueTracking's isKnownNeverNaN treats undef as not-nan, but only when it appears as an element of a vector constant.

Petar.Avramovic retitled this revision from AMDGPU/GlobalISel: Treat undef as KnownNeverNaN to AMDGPU/GlobalISel: Do not fcanonicalize const splat padded with undef.
Petar.Avramovic edited the summary of this revision. (Show Details)

Move to isCanonicalized and post-legalizer combiner.

foad added inline comments.Jun 17 2021, 6:34 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
9853

What if SplatReg is a signaling nan here?

Fix cases of SNaN and QNaN constant splats padded with undef, also add test for them.

foad added inline comments.Jun 17 2021, 7:27 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
9853

What about NaNs? This seems like it should be combined with the G_FCONSTANT case above.

Merge with G_FCONSTANT case above.

arsenm added inline comments.Jun 29 2021, 3:06 PM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
9844

If the matcher is finding it's a constant, why does it return the register instead of the direct APFloat?

Use matcher that returns both register and APFloat from G_FCONSTANT.

arsenm accepted this revision.Jul 9 2021, 2:13 PM
This revision is now accepted and ready to land.Jul 9 2021, 2:13 PM
This revision was landed with ongoing or failed builds.Dec 3 2021, 4:02 AM
This revision was automatically updated to reflect the committed changes.