Page MenuHomePhabricator

[GlobalISel][Legalizer] Restore eraseFromParentAndMarkDBGValuesForRemoval() for CallLowering artifacts.
AbandonedPublic

Authored by jackoalan on Oct 17 2021, 5:16 PM.

Details

Summary

D109750 was created to improve performance by avoiding the expense of eraseFromParentAndMarkDBGValuesForRemoval for instructions that are known to not be emitted by IR translation. However, this does not account for call lowering which is indirectly invoked by IR translation. Call lowering has reachability of G_UNMERGE_VALUES, G_MERGE_VALUES, G_CONCAT_VECTORS, and G_BUILD_VECTOR so these should be removed from this exclusion test.

Compiling a function with an argument wider than the native machine types results in G_MERGE_VALUES which is used by DBG_VALUE. This trivial case on i386 causes a validation failure post-legalization:

int test_dbg_trunc(unsigned long long a) { return a; }

Diff Detail

Event Timeline

jackoalan created this revision.Oct 17 2021, 5:16 PM
jackoalan requested review of this revision.Oct 17 2021, 5:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2021, 5:16 PM

Please see the discussion here: https://groups.google.com/g/llvm-dev/c/R9lSoAqh5e4

The summary is that this isn't technically incorrect. We can address your test case either by fixing the assert in the instruction selector to not care if the vreg being checked is only used by DBG_VALUE instructions, or we can do it by having a pass over all the VRegs after the legalizer runs. If we go for the second, we might as well always use eraseFromParent() anyway.

All, that makes much more sense. If performance is the objective, allowing the dangling use and relaxing the asserts might be the better option.

jackoalan abandoned this revision.Oct 18 2021, 2:56 PM

Abandoning this for now, since the solution would be completely rethinking how debug value uses are handled.