AMDGPU/SI: Don't handle a loop if there is no loop at all for a terminator BB.

[cfang]

Summary

This will avoid segmentation fault to access the loop fields.

[arsenm]

The comments I think are copied from another test and probably not relevant for this, so should be removed.

The test function should also have a better name about the situation with unreachables

[arsenm]

The test also should run the IR pass itself and check for the inserted control flow intrinsics

[arsenm]

Can you also add a version of the test that uses ret instead of unreachable?

[cfang]

In D22021#474656, @arsenm wrote:

The test also should run the IR pass itself and check for the inserted control flow intrinsics

For this patch, we give up "HandleLoop" if there is no loop. So I am not sure what to check for the inserted intrinsics.
Without the patch, there will be segfault.

[cfang]

Update based on review comments:

1. remove irrelevant comment;
2. add OPT test to check the llvm.amdgcn.loop intrinsic is not added;
3. add test that uses "ret" instruction instead of "unreachable".

[arsenm]

LGTM, though it might be a good idea to add some code before unreachable / ret in this one too in case something decides that it can de-duplicate blocks someday

[cfang]

Patch committed to LLVM trunk

commit 539fec5dc2cb0b0931ab47f4b705419e8f18bd34
Author: Changpeng Fang <[e-mail]>
Date: Thu Jul 28 23:01:45 2016 +0000

AMDGPU/SI: Don't handle a loop if there is no loop at all for a terminator BB.

Differential Revision: http://reviews.llvm.org/D22021

Reviewed by: arsenm

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@277073 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/AMDGPU/SIAnnotateControlFlow.cpp

 }
 
 /// \brief Handle a back edge (loop)
 void SIAnnotateControlFlow::handleLoop(BranchInst *Term) {
   if (isUniform(Term)) {
     return;
   }
 
   BasicBlock *BB = Term->getParent();
   llvm::Loop *L = LI->getLoopFor(BB);
+  if (!L)
+    return;
   BasicBlock *Target = Term->getSuccessor(1);
   PHINode *Broken = PHINode::Create(Int64, 0, "", &Target->front());
 
   Value *Cond = Term->getCondition();
   Term->setCondition(BoolTrue);
   Value *Arg = handleLoopCondition(Cond, Broken, L, Term);
 
   for (pred_iterator PI = pred_begin(Target), PE = pred_end(Target);
        PI != PE; ++PI) {
 

test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll

+; RUN: opt -mtriple=amdgcn-- -S -structurizecfg -si-annotate-control-flow %s | FileCheck -check-prefix=OPT %s
+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+
+
+; OPT-LABEL: @annotate_unreachable_noloop(
+; OPT-NOT: call i1 @llvm.amdgcn.loop
+
+; GCN-LABEL: {{^}}annotate_unreachable_noloop:
+; GCN: s_cbranch_vccnz
+; GCN-NOT: s_endpgm
+; GCN: .Lfunc_end0
+define void @annotate_unreachable_noloop(<4 x float> addrspace(1)* noalias nocapture readonly %arg) #0 {
+bb:
+  %tmp = tail call i32 @llvm.amdgcn.workitem.id.x()
+  br label %bb1
+
+bb1:                                              ; preds = %bb
+  %tmp2 = sext i32 %tmp to i64
+  %tmp3 = getelementptr inbounds <4 x float>, <4 x float> addrspace(1)* %arg, i64 %tmp2
+  %tmp4 = load <4 x float>, <4 x float> addrspace(1)* %tmp3, align 16
+  br i1 undef, label %bb5, label %bb3
+
+bb3:                                              ; preds = %bb1
+  %tmp6 = extractelement <4 x float> %tmp4, i32 2
+  %tmp7 = fcmp olt float %tmp6, 0.000000e+00
+  br i1 %tmp7, label %bb4, label %bb5 ; crash goes away if these are swapped
+
+bb4:                                              ; preds = %bb3
+  unreachable
+
+bb5:                                              ; preds = %bb3, %bb1
+  unreachable
+}
+
+
+; OPT-LABEL: @annotate_ret_noloop(
+; OPT-NOT: call i1 @llvm.amdgcn.loop
+
+; GCN-LABEL: {{^}}annotate_ret_noloop:
+; GCN: s_and_b64 vcc
+; GCN: s_cbranch_vccnz
+; GCN: s_endpgm
+; GCN: .Lfunc_end1
+define void @annotate_ret_noloop(<4 x float> addrspace(1)* noalias nocapture readonly %arg) #0 {
+bb:
+  %tmp = tail call i32 @llvm.amdgcn.workitem.id.x()
+  br label %bb1
+
+bb1:                                              ; preds = %bb
+  %tmp2 = sext i32 %tmp to i64
+  %tmp3 = getelementptr inbounds <4 x float>, <4 x float> addrspace(1)* %arg, i64 %tmp2
+  %tmp4 = load <4 x float>, <4 x float> addrspace(1)* %tmp3, align 16
+  br i1 undef, label %bb5, label %bb3
+
+bb3:                                              ; preds = %bb1
+  %tmp6 = extractelement <4 x float> %tmp4, i32 2
+  %tmp7 = fcmp olt float %tmp6, 0.000000e+00
+  br i1 %tmp7, label %bb4, label %bb5 ; crash goes away if these are swapped
+
+bb4:                                              ; preds = %bb3
+  ret void
+
+bb5:                                              ; preds = %bb3, %bb1
+  ret void
+}
+
+
+declare i32 @llvm.amdgcn.workitem.id.x() #1
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind readnone }