[PATCH] D104063: [SimplifyCFG] avoid crash on degenerate loop

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D104063#2811595 <https://reviews.llvm.org/D104063#2811595>, @lebedev.ri wrote:

> And what happens when there's more than one PHI node in the block?

If I'm seeing the code correctly, we can't get into trouble on anything other than the pattern in the test. That's because we limit the transform to a block with at most 2 phis (line 2723). If the problematic phi was 2nd, we'd escape without trying to touch the freed instruction.

But it's a valid concern (the assumption of 2 phis could change some day), so I agree, we should do better...

> I think the extra missing check is `!isa<Instruction>(IfCond) || cast<Instruction>(IfCond)->getParent() != BB`.

But this won't work unless I've misunderstood the suggestion - the condition could be a function argument. Here's the first failing test that I found with that check:

  define i32 @FoldTwoEntryPHINode(i1 %C, i32 %V1, i32 %V2, i16 %V3) {
  entry:
          br i1 %C, label %then, label %else, !prof !0, !unpredictable !1
  then:
          %V4 = or i32 %V2, %V1
          br label %Cont
  else:
          %V5 = sext i16 %V3 to i32
          br label %Cont
  Cont:
          %V6 = phi i32 [ %V5, %else ], [ %V4, %then ]
          call i32 @FoldTwoEntryPHINode( i1 false, i32 0, i32 0, i16 0 )
          ret i32 %V1
  }

We still want that to fold to:

  ; CHECK-LABEL: @FoldTwoEntryPHINode(
  ; CHECK-NEXT:  entry:
  ; CHECK-NEXT:  %V5 = sext i16 %V3 to i32
  ; CHECK-NEXT:  %V4 = or i32 %V2, %V1
  ; CHECK-NEXT:  %V6 = select i1 %C, i32 %V4, i32 %V5, !prof !0, !unpredictable !1
  ; CHECK-NEXT:  %0 = call i32 @FoldTwoEntryPHINode(i1 false, i32 0, i32 0, i16 0)
  ; CHECK-NEXT:  ret i32 %V1

  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104063/new/

https://reviews.llvm.org/D104063