This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] gfx1010 wave32 icmp/fcmp intrinsic changes for wave32
ClosedPublic

Authored by rampitec on Jun 13 2019, 3:06 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.Jun 13 2019, 3:06 PM
arsenm added inline comments.Jun 13 2019, 3:13 PM
lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
252 ↗(On Diff #204635)

This one should query the wavesize?

lib/Target/AMDGPU/SIInstructions.td
606–614 ↗(On Diff #204635)

I don't think these should ever get here? These should have been turned into AMDGPUSsetcc

rampitec marked 2 inline comments as done.Jun 13 2019, 3:19 PM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
252 ↗(On Diff #204635)

They actually don't, they are anyint and 64 fits it.

lib/Target/AMDGPU/SIInstructions.td
606–614 ↗(On Diff #204635)

That's an inplace replacement. We can explore if it still used or not, but keeping just wave64 version is clearly wrong.

arsenm accepted this revision.Jun 13 2019, 3:51 PM

LGTM

This revision is now accepted and ready to land.Jun 13 2019, 3:51 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2019, 4:44 PM

HI,

This change introduces a bunch of failures with CTS on GFX8/GFX9. Looks like the IR validation fail now, see below:

$ RADV_DEBUG=checkir,nocache ./deqp-vk --deqp-case=dEQP-VK.glsl.derivate.dfdxfine.dynamic_loop.vec2_highp
Writing test log into TestResults.qpa
dEQP Core git-c2a38f956feeb2e141c76829c6ecb41c9f68d253 (0xc2a38f95) starting..

target implementation = 'Default'

Test case 'dEQP-VK.glsl.derivate.dfdxfine.dynamic_loop.vec2_highp'..
Intrinsic name not mangled correctly for type arguments! Should be: llvm.amdgcn.icmp.i64.i32
i64 (i32, i32, i32)* @llvm.amdgcn.icmp.i32
in function main
LLVM ERROR: Broken function found, compilation aborted!

Can you investigate please?
Thanks!

HI,

This change introduces a bunch of failures with CTS on GFX8/GFX9. Looks like the IR validation fail now, see below:

$ RADV_DEBUG=checkir,nocache ./deqp-vk --deqp-case=dEQP-VK.glsl.derivate.dfdxfine.dynamic_loop.vec2_highp
Writing test log into TestResults.qpa
dEQP Core git-c2a38f956feeb2e141c76829c6ecb41c9f68d253 (0xc2a38f95) starting..

target implementation = 'Default'

Test case 'dEQP-VK.glsl.derivate.dfdxfine.dynamic_loop.vec2_highp'..
Intrinsic name not mangled correctly for type arguments! Should be: llvm.amdgcn.icmp.i64.i32
i64 (i32, i32, i32)* @llvm.amdgcn.icmp.i32
in function main
LLVM ERROR: Broken function found, compilation aborted!

Can you investigate please?
Thanks!

Intrinsic mangling has changed. Normally llvm can handle it gracefully. How do you load a module? Can you attach a testcase please? I assume autoupgrade code may be needed, but we need to understand the workflow of RADV.

Do we need to update the intrinsic name from mesa?
Btw, it's not related to RADV, RadeonSI is probably affected too because the LLVM backend is common.

Yes, thanks! If module with old name is created on the fly it has no chance to upgrade. Thus clang was updated and Mesa too.