This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][GISel] Allow VReg srcs in (build_vector undef, i16) pattern
ClosedPublic

Authored by Pierre-vh on Oct 19 2022, 2:09 AM.

Details

Reviewers
arsenm
foad
Summary

Fixes a regression in v_mad_mixhi_f16_f16lo_f16lo_f16lo_undeflo_clamp_precvt.

Depends on D134354

Diff Detail

Event Timeline

Pierre-vh created this revision.Oct 19 2022, 2:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2022, 2:09 AM
Pierre-vh requested review of this revision.Oct 19 2022, 2:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2022, 2:09 AM
arsenm accepted this revision.Oct 19 2022, 8:01 AM
This revision is now accepted and ready to land.Oct 19 2022, 8:01 AM
foad added inline comments.Oct 19 2022, 8:35 AM
llvm/lib/Target/AMDGPU/SIInstructions.td
2740–2741

Why does this pattern use SReg_32? That doesn't make any sense to me.

arsenm added inline comments.Oct 19 2022, 9:43 AM
llvm/lib/Target/AMDGPU/SIInstructions.td
2740–2741

The output should change to match.

It makes sense for the old DAG select everything to scalar and hack up to vector later approach

foad added inline comments.Oct 19 2022, 11:45 AM
llvm/lib/Target/AMDGPU/SIInstructions.td
2740–2741

Well I still don't get why it ever made sense. I know we used to select SALU instructions by default, but this is a VALU instruction with an SGPR operand. I thought we always used VGPR operands by default for VALU instructions, to avoid having to think about constant bus restrictions until SIFoldOperands.

arsenm added inline comments.Oct 19 2022, 11:47 AM
llvm/lib/Target/AMDGPU/SIInstructions.td
2740–2741

The register class in the pattern used to mean less and was practically an alias for the type. There was a finalize ISel hook to fixup constant bus violations

Pierre-vh closed this revision.Nov 6 2022, 11:53 PM

Done as part of D136459 - results on codegen in D134354 are identical.