[PATCH] D24190: Do not consider subreg defs as reads when computing subrange liveness

[Quentin Colombet via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

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


================
Comment at: lib/CodeGen/LiveIntervalAnalysis.cpp:512
@@ -511,3 +511,3 @@
     if (UseMI->isDebugValue() || !MO.readsReg())
       continue;
     // Maybe the operand is for a subregister we don't care about.
----------------
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.

================
Comment at: lib/CodeGen/LiveRangeCalc.cpp:173
@@ -171,3 +172,3 @@
       MO.setIsKill(false);
-    if (!MO.readsReg())
+    if (!MO.readsReg() || (IsSubRange && MO.isDef()))
       continue;
----------------
Add a comment saying that if we readsReg, but MO.isDef(), that means for a subrange that this is not a true use. 

================
Comment at: test/CodeGen/AMDGPU/scheduler-liveness-1.ll:2
@@ +1,3 @@
+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck %s
+; Check for a sane output. This testcase used to crash.
+; CHECK: image_sample_c
----------------
Add more detail on what we are checking for.
In particular, why was the crash happening.

================
Comment at: test/CodeGen/AMDGPU/scheduler-liveness-2.ll:2
@@ +1,3 @@
+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck %s
+; Check for a sane output. This testcase used to crash.
+; CHECK: tbuffer_store_format_x
----------------
Edit.

================
Comment at: test/CodeGen/AMDGPU/unigine-liveness-crash.ll:3
@@ +2,3 @@
+; Check for a sane output. This testcase used to crash.
+; CHECK: image_sample_c
+
----------------
Ditto


Repository:
  rL LLVM

https://reviews.llvm.org/D24190