This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Select llvm.amdgcn.div.scale
ClosedPublic

Authored by arsenm on Apr 6 2020, 6:13 AM.

Details

Diff Detail

Event Timeline

arsenm created this revision.Apr 6 2020, 6:13 AM
kerbowa accepted this revision.Apr 6 2020, 8:38 AM

LGTM
I had this implemented as well, but was trying to get the SD tests to pass first.

This revision is now accepted and ready to land.Apr 6 2020, 8:38 AM
foad added inline comments.Apr 8 2020, 12:33 AM
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
825

This doesn't seem to match the comment in IntrinsicsAMDGPU.td which says "0 = first, 1 = second" (where I assume "first" means "numerator"...).

kerbowa added inline comments.Apr 9 2020, 2:42 AM
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
825

It looks like this is copying the SD lowering to the intermediate node for this intrinsic which doesn't match up to that comment in IntrinsicsAMDGPU.td. So the GlobalISel tests at least match the DAG version. Was the intrinsic not being used directly before? I think it needs to be changed in both places or the comment should be updated.

arsenm marked an inline comment as done.Apr 9 2020, 9:29 AM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
825

I'm not aware of any users of the intrinsic other than the GlobalISel legalization.