Page MenuHomePhabricator

Do not consider subreg defs as reads when computing subrange liveness
ClosedPublic

Authored by kparzysz on Sep 2 2016, 7:07 AM.

Details

Summary

The problems that that D23879 and D23942 were trying to fix were actually caused by incorrect subrange calculation. Fixing this issue makes all the associated testcases compile successfully.

Diff Detail

Repository
rL LLVM

Event Timeline

kparzysz updated this revision to Diff 70156.Sep 2 2016, 7:07 AM
kparzysz retitled this revision from to Do not consider subreg defs as reads when computing subrange liveness.
kparzysz updated this object.
kparzysz added reviewers: qcolombet, MatzeB.
kparzysz set the repository for this revision to rL LLVM.
qcolombet accepted this revision.Sep 2 2016, 11:01 AM
qcolombet edited edge metadata.

Hi Krzysztof,

Thanks for getting to the bottom of this!

LGTM with nitpicks:

  • Use 'opt -instnamer' on the bitcode test cases to get rid of the [0-9]+ variables.
  • One continue is now useless, see the inlined comment.

Cheers,
-Quentin

lib/CodeGen/LiveIntervalAnalysis.cpp
512 ↗(On Diff #70156)

We should get only operands that reads reg and is not a debug value.
I.e., turn that into an assert or leave it out.

lib/CodeGen/LiveRangeCalc.cpp
173 ↗(On Diff #70156)

Add a comment saying that if we readsReg, but MO.isDef(), that means for a subrange that this is not a true use.

test/CodeGen/AMDGPU/scheduler-liveness-1.ll
2 ↗(On Diff #70156)

Add more detail on what we are checking for.
In particular, why was the crash happening.

test/CodeGen/AMDGPU/scheduler-liveness-2.ll
2 ↗(On Diff #70156)

Edit.

test/CodeGen/AMDGPU/unigine-liveness-crash.ll
3 ↗(On Diff #70156)

Ditto

This revision is now accepted and ready to land.Sep 2 2016, 11:01 AM
This revision was automatically updated to reflect the committed changes.