This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/InsertWaitcnts: Remove the dependence on MachineLoopInfo
ClosedPublic

Authored by nhaehnle on Nov 7 2018, 2:17 PM.

Details

Summary

MachineLoopInfo cannot be relied on for correctness, because it cannot
properly recognize loops in irreducible control flow which can be
introduced by late machine basic block optimization passes. See the new
test case for the reduced form of an example that occurred in practice.

Use a simple fixpoint iteration instead.

In order to facilitate this change, refactor WaitcntBrackets so that it
only tracks pending events and registers, rather than also maintaining
state that is relevant for the high-level algorithm. Various accessor
methods can be removed or made private as a consequence.

Affects (in radv):

  • dEQP-VK.glsl.loops.special.{for,while}_uniform_iterations.select_iteration_count_{fragment,vertex}

Fixes: r345719 ("AMDGPU: Rewrite SILowerI1Copies to always stay on SALU")

Diff Detail

Repository
rL LLVM

Event Timeline

nhaehnle created this revision.Nov 7 2018, 2:17 PM
This revision is now accepted and ready to land.Nov 22 2018, 9:37 AM
This revision was automatically updated to reflect the committed changes.

Hi Nicolai,

This change introduced a regression in The Witcher 3, see https://bugs.freedesktop.org/show_bug.cgi?id=108935

Thanks.

Did you get a chance to look into this?

Thanks!

This change introduced a regression in The Witcher 3, see https://bugs.freedesktop.org/show_bug.cgi?id=108935

Hi Samuel,

Would you be able to test https://reviews.llvm.org/D55602 to see if it resolves the issue?

Thanks,
Carl

Hi,

Yes, it does fix the issue. Thanks!