This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Check if type transforms to i16 (VI+) when getting AMDGPUISD::FFBH_U32
ClosedPublic

Authored by kzhuravl on Oct 19 2016, 4:42 PM.

Details

Summary

This will prevent following regression when enabling i16 support (D18049):

test/CodeGen/AMDGPU/ctlz.ll
test/CodeGen/AMDGPU/ctlz_zero_undef.ll

Diff Detail

Event Timeline

kzhuravl updated this revision to Diff 75252.Oct 19 2016, 4:42 PM
kzhuravl retitled this revision from to [AMDGPU] Promote ctlz (i1, i16] intrinsic to i32.
kzhuravl updated this object.
kzhuravl added a reviewer: tstellarAMD.
kzhuravl added a subscriber: llvm-commits.
lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
482

Shouldn't we only be doing this for uniform ops? Otherwise the SelectionDAG should be able to do the promotion.

kzhuravl updated this revision to Diff 75500.Oct 21 2016, 4:12 PM
kzhuravl retitled this revision from [AMDGPU] Promote ctlz (i1, i16] intrinsic to i32 to [AMDGPU] Check if type transforms to i16 (VI+) when getting AMDGPUISD::FFBH_U32.
kzhuravl updated this object.
kzhuravl edited edge metadata.

We do not need to promote it. Fixed during combining.

arsenm added inline comments.Oct 26 2016, 12:56 PM
lib/Target/AMDGPU/AMDGPUISelLowering.cpp
2461 ↗(On Diff #75500)

Subtarget->hasI16

2468 ↗(On Diff #75500)

Variable name probably shouldn't match the node type name

lib/Target/AMDGPU/AMDGPUISelLowering.h
33 ↗(On Diff #75500)

I think SDLoc is supposed to be passed by const reference now

kzhuravl added inline comments.Oct 26 2016, 1:24 PM
lib/Target/AMDGPU/AMDGPUISelLowering.cpp
2461 ↗(On Diff #75500)

This will involve moving has16BitInsts() from SISubtarget to AMDGPUSubtarget. Are there any objections?

kzhuravl updated this revision to Diff 76039.Oct 27 2016, 8:41 AM
kzhuravl edited edge metadata.
kzhuravl marked 4 inline comments as done.

Address review feedback

tstellarAMD accepted this revision.Nov 1 2016, 10:34 AM
tstellarAMD edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Nov 1 2016, 10:34 AM
This revision was automatically updated to reflect the committed changes.