This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Change intrinsic ID for _L to _LZ opt
ClosedPublic

Authored by arsenm on Mar 30 2020, 1:47 PM.

Details

Summary

Still should handle the other case changes the opcode this way

Diff Detail

Event Timeline

arsenm created this revision.Mar 30 2020, 1:47 PM

What is the reason to do this during legalization?
Could we look for a constant zero argument during instruction selection?

Then we could also remove the lz variants of the intrinsics and simply encode this as a constant zero argument.

What is the reason to do this during legalization?
Could we look for a constant zero argument during instruction selection?

I think any operand changes should be done during legalization, and selection should be relatively simple. We already do more of this type of conversion during legalize, so I think it makes sense to consolidate it there.

Then we could also remove the lz variants of the intrinsics and simply encode this as a constant zero argument.

That's what it does now

Flakebi accepted this revision.Mar 31 2020, 6:18 AM

I think any operand changes should be done during legalization, and selection should be relatively simple. We already do more of this type of conversion during legalize, so I think it makes sense to consolidate it there.

Thanks for the explanation, looks good to me then.

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
3730–3731

Can you update the comment to reflect the changes?

llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.sample.ltolz.a16.ll
27

The tests are outdated, they should not contain the 0 lod argument.

This revision is now accepted and ready to land.Mar 31 2020, 6:18 AM