This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Eliminate no effect instructions before s_endpgm
ClosedPublic

Authored by rampitec on Aug 10 2017, 10:06 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.Aug 10 2017, 10:06 AM

Is there a more general dead code elimination strategy that could be used here? It seems like we always want to eliminate writes to EXEC that have no users.

Is there a more general dead code elimination strategy that could be used here? It seems like we always want to eliminate writes to EXEC that have no users.

So far DCE cannot do anything of that.

arsenm edited edge metadata.Aug 14 2017, 11:30 AM

Needs MIR test

lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
114 ↗(On Diff #110605)

SmallSet?

120 ↗(On Diff #110605)

succ_empty

141–143 ↗(On Diff #110605)

hasOrderedMemoryRef

164 ↗(On Diff #110605)

Braces

166 ↗(On Diff #110605)

It's seems overly aggressive to be trying to prune other blocks here. Should these types of instructions be sunk or blocks merged earlier?

rampitec updated this revision to Diff 111045.Aug 14 2017, 12:04 PM
rampitec marked 3 inline comments as done.

Addressed review comments. Mir test pending.

lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
114 ↗(On Diff #110605)

SmallSet does not provide a way to iterate over members in the set.

166 ↗(On Diff #110605)

It does not mean they can be sunk. Two predecessors may have different tailing instructions, but all can be dead given the path to s_endpgm. Also some instructions are eliminated from preceding blocks by the very same pass which would then allow to merge blocks, but only after this pass finished.

rampitec updated this revision to Diff 111070.Aug 14 2017, 2:08 PM

Added mir test.

arsenm accepted this revision.Aug 15 2017, 4:58 PM

LGTM. The searching through all predecessors seems overly aggressive to me though,

This revision is now accepted and ready to land.Aug 15 2017, 4:58 PM
This revision was automatically updated to reflect the committed changes.