[PATCH] D25306: AMDGPU: Fix use-after-free in SIOptimizeExecMasking

[Nicolai Hähnle via llvm-commits]

nhaehnle created this revision.
nhaehnle added reviewers: arsenm, tstellarAMD.
nhaehnle added a subscriber: llvm-commits.
Herald added subscribers: tony-tye, yaxunl, wdng, kzhuravl.

There was a bug with sequences like

  s_mov_b64 s[0:1], exec
  s_and_b64 s[2:3]<def>, s[0:1], s[2:3]<kill>
  ...
  s_mov_b64_term exec, s[2:3]

because s[2:3] was defined and used in the same instruction, ending up with
SaveExecInst inside OtherUseInsts.

Note that the test case also exposes an unrelated bug.


https://reviews.llvm.org/D25306

Files:
  lib/Target/AMDGPU/SIOptimizeExecMasking.cpp
  test/CodeGen/AMDGPU/branch-condition-and.ll


Index: test/CodeGen/AMDGPU/branch-condition-and.ll
===================================================================
--- /dev/null
+++ test/CodeGen/AMDGPU/branch-condition-and.ll
@@ -0,0 +1,39 @@
+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+
+; This used to crash because during intermediate control flow lowering, there
+; was a sequence
+;       s_mov_b64 s[0:1], exec
+;       s_and_b64 s[2:3], s[0:1], s[2:3] ; def & use of the same register pair
+;       ...
+;       s_mov_b64_term exec, s[2:3]
+; that was not treated correctly.
+;
+; GCN-LABEL: {{^}}ham:
+; GCN-DAG: v_cmp_lt_f32_e64 [[OTHERCC:s\[[0-9]+:[0-9]+\]]],
+; GCN-DAG: v_cmp_lt_f32_e32 vcc,
+; GCN: s_and_b64 [[AND:s\[[0-9]+:[0-9]+\]]], vcc, [[OTHERCC]]
+; GCN: s_and_saveexec_b64 [[SAVED:s\[[0-9]+:[0-9]+\]]], [[AND]]
+; GCN: s_xor_b64 [[SAVED]], exec, [[SAVED]]
+;
+; TODO: The following sequence is a bug (missing s_endpgm)!
+;
+; GCN: s_branch BB0_1
+; GCN: BB0_1:
+; GCN-NEXT: .Lfunc_end0:
+define amdgpu_ps void @ham(float %arg, float %arg1) #0 {
+bb:
+  %tmp = fcmp ogt float %arg, 0.000000e+00
+  %tmp2 = fcmp ogt float %arg1, 0.000000e+00
+  %tmp3 = and i1 %tmp, %tmp2
+  br i1 %tmp3, label %bb4, label %bb5
+
+bb4:                                              ; preds = %bb
+  unreachable
+
+bb5:                                              ; preds = %bb
+  ret void
+}
+
+attributes #0 = { nounwind readonly "InitialPSInputAddr"="36983" }
+attributes #1 = { nounwind readnone }
Index: lib/Target/AMDGPU/SIOptimizeExecMasking.cpp
===================================================================
--- lib/Target/AMDGPU/SIOptimizeExecMasking.cpp
+++ lib/Target/AMDGPU/SIOptimizeExecMasking.cpp
@@ -248,14 +248,17 @@
         if (J->readsRegister(CopyFromExec, TRI)) {
           SaveExecInst = &*J;
           DEBUG(dbgs() << "Found save exec op: " << *SaveExecInst << '\n');
+          continue;
         } else {
           DEBUG(dbgs() << "Instruction does not read exec copy: " << *J << '\n');
           break;
         }
       }
 
-      if (SaveExecInst && J->readsRegister(CopyToExec, TRI))
+      if (SaveExecInst && J->readsRegister(CopyToExec, TRI)) {
+        assert(SaveExecInst != &*J);
         OtherUseInsts.push_back(&*J);
+      }
     }
 
     if (!SaveExecInst)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25306.73739.patch
Type: text/x-patch
Size: 2403 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161006/d35de809/attachment.bin>