[PATCH] D81004: [MemorySSA] Skip phi translation for phis in cycles (WIP).

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

Thank you for the testcase, this does show the issue very well.
The solution I think it's incomplete. This can occur in any loop, not just for a trivial phi. For example with just adding an if/else in the loop.

  define i32 @test1(i32* noalias %ptr) {
  entry:
    %s1.ptr = getelementptr inbounds [2 x i16], [2 x i16]* @c, i64 0, i64 0
  ; 1 = MemoryDef(liveOnEntry)
    store i16 1, i16* %s1.ptr, align 2, !tbaa !0
    br label %for.body
  
  for.body:                                         ; preds = %merge.body, %entry
  ; 4 = MemoryPhi({entry,1},{merge.body,2})
    %storemerge2 = phi i32 [ 1, %entry ], [ %dec, %merge.body ]
    %idxprom1 = zext i32 %storemerge2 to i64
    %arrayidx = getelementptr inbounds [2 x i16], [2 x i16]* @c, i64 0, i64 %idxprom1
  ; MemoryUse(liveOnEntry)
    %lv = load i16, i16* %arrayidx, align 2, !tbaa !0
    %conv = sext i16 %lv to i32
  ; 2 = MemoryDef(4)
    store i32 %conv, i32* %ptr, align 4, !tbaa !4
    %dec = add nsw i32 %storemerge2, -1
    %cmpif = icmp sgt i32 %storemerge2, 1
    br i1 %cmpif, label %if.body, label %else.body
  
  if.body:                                          ; preds = %for.body
    br label %merge.body
  
  else.body:                                        ; preds = %for.body
    br label %merge.body
  
  merge.body:                                       ; preds = %else.body, %if.body
    %cmp = icmp sgt i32 %storemerge2, 0
    br i1 %cmp, label %for.body, label %for.end
  
  for.end:                                          ; preds = %merge.body
    %s2.ptr = getelementptr inbounds [2 x i16], [2 x i16]* @c, i64 0, i64 0
  ; 3 = MemoryDef(2)
    store i16 0, i16* %s2.ptr, align 2, !tbaa !0
    ret i32 0
  }

A  conservative approach would be: "if this block is part of a loop".
I'm not sure how to make this check cheap however.

A potential solution is passing LoopInfo if available and check if `OriginalAccess` is in the blocks of any Loop. But I expect this check can get expensive with a large number of loops/blocks, plus LI is generally available in loop passes, so when LI is available the answer is likely skip PhiTranslation without checking the blocks at all..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81004