This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Fix selection of s1/s16 G_[F]CONSTANT
ClosedPublic

Authored by arsenm on Jul 27 2020, 5:20 AM.

Details

Summary

The code to determine the value size was overcomplicated and only
correct in the case where the result register already had a register
class assigned. We can always take the size directly from the
register's type.

Diff Detail

Event Timeline

arsenm created this revision.Jul 27 2020, 5:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2020, 5:20 AM
foad added a comment.Aug 17 2020, 6:27 AM

It's hard to see which test diffs are relevant. Could you commit some changes (like tweaks to the implicit operands on S_ENDPGM) separately?

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fconstant.mir
140–141

I'm confused - how are these lines testing anything different than lines 134-135?

arsenm added inline comments.Aug 17 2020, 6:30 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fconstant.mir
140–141

The %2, %3 cases already worked because they have uses on the copy instruction, which constrained the source register class so this worked. The %4/%5 cases do not have uses that will be constrained to a regclass, so this would hit the broken path for getting the register size

It's hard to see which test diffs are relevant. Could you commit some changes (like tweaks to the implicit operands on S_ENDPGM) separately?

These will fail to select and require the -global-isel-abort=0 hack, which I'm trying to stop using

foad accepted this revision.Aug 18 2020, 2:22 AM
This revision is now accepted and ready to land.Aug 18 2020, 2:22 AM