This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Optimize _L image intrinsic to _LZ when lod is zero
ClosedPublic

Authored by rtaylor on Jul 18 2018, 6:58 AM.

Details

Summary

Add _L to _LZ image intrinsic table mapping to table gen.
In ISelLowering check if image intrinsic has lod and if it's equal
to zero, if so remove lod and change opcode to equivalent mapped _LZ.

Change-Id: Ie24cd7e788e2195d846c7bd256151178cbb9ec71

Diff Detail

Event Timeline

rtaylor created this revision.Jul 18 2018, 6:58 AM
arsenm added inline comments.Jul 18 2018, 7:45 AM
lib/Target/AMDGPU/SIISelLowering.cpp
4588

Does this need to check that it is positive zero?

Should this be done in an IR pass instead?

Should this be done in an IR pass instead?

This could be done in the IR but this avoids a long(er) switch statement (# of combinations) and keeps more of the image intrinsic work in the same place, which seemed per conversations the best way to go. Is there some advantage to moving this to the IR (InstCombine for example)?

lib/Target/AMDGPU/SIISelLowering.cpp
4588

I used isZero to cover both pos and neg zero.

I don't think doing this as an IR pass has any advantage, so this is fine.

Please add tests for gather intrinsics as well, apart from that it looks good to me.

lib/Target/AMDGPU/SIISelLowering.cpp
4588

That's an interesting question. Since LOD is clamped at 0.0 anyway, the only possible difference could come from switching between magnification and minification filters. The OpenGL spec says that the magnification filter is used when LOD is less than or equal to 0.0 (and actually even when it's <= 0.5 for nearest mipmap minification). So this test could be relaxed to check whether ConstantLod is <= 0.0.

rtaylor updated this revision to Diff 157961.Jul 30 2018, 7:42 AM

Added gather4 test cases.
Added 'isNegative' to _L to _LZ check to include <= 0.0

nhaehnle accepted this revision.Jul 31 2018, 8:08 AM

Thanks! LGTM

This revision is now accepted and ready to land.Jul 31 2018, 8:08 AM
This revision was automatically updated to reflect the committed changes.