[PATCH] D108043: [NFCI][IndVars] rewriteLoopExitValues(): nowadays SCEV should not change `GEP` base pointer

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri reopened this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

In D108043#2946441 <https://reviews.llvm.org/D108043#2946441>, @bjope wrote:

> I just wrote https://bugs.llvm.org/show_bug.cgi?id=51490 which triggered this assert (using -Oz and legacy PM).

Reduced this looks like the following:

  ; ModuleID = 'input.ll'
  source_filename = "/tmp/csmith-2231729227.c"
  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
  target triple = "x86_64-unknown-linux-gnu"
  
  @g_2168 = external global [4 x [6 x i32]], align 16
  @g_1150 = external global i32*, align 8
  
  define internal fastcc void @func_2() unnamed_addr {
  lbl_2898.preheader:
    br label %lbl_2898
  
  lbl_2898.loopexit:                                ; preds = %for.cond884
    %.lcssa = phi i32* [ %0, %for.cond884 ]
    store i32* %.lcssa, i32** @g_1150, align 8, !tbaa !0
    br label %lbl_2898
  
  lbl_2898:                                         ; preds = %lbl_2898.loopexit, %lbl_2898.preheader
    %g_1150.promoted = load i32*, i32** @g_1150, align 8, !tbaa !0
    br label %for.cond884
  
  for.cond884:                                      ; preds = %for.end987, %lbl_2898
    %0 = phi i32* [ getelementptr inbounds ([4 x [6 x i32]], [4 x [6 x i32]]* @g_2168, i64 0, i64 3, i64 1), %for.end987 ], [ %g_1150.promoted, %lbl_2898 ]
    %storemerge9 = phi i16 [ %add990, %for.end987 ], [ 0, %lbl_2898 ]
    %cmp886 = icmp slt i16 %storemerge9, 3
    br i1 %cmp886, label %for.body888, label %lbl_2898.loopexit
  
  for.body888:                                      ; preds = %for.cond884
    br label %for.cond918
  
  for.cond918:                                      ; preds = %for.body888
    br label %for.end926
  
  for.end926:                                       ; preds = %for.cond918
    br label %for.cond936
  
  for.cond936:                                      ; preds = %for.end926
    br label %for.end987
  
  for.end987:                                       ; preds = %for.cond936
    %add990 = add i16 %storemerge9, 1
    br label %for.cond884
  }
  
  !0 = !{!1, !1, i64 0}
  !1 = !{!"any pointer", !2, i64 0}
  !2 = !{!"omnipotent char", !3, i64 0}
  !3 = !{!"Simple C/C++ TBAA"}

  INDVARS: Turn to unsigned comparison:   %cmp886 = icmp slt i16 %storemerge9, 3
  rewriteLoopExitValues: AfterLoopVal = i32* getelementptr inbounds ([4 x [6 x i32]], [4 x [6 x i32]]* @g_2168, i64 0, i64 3, i64 1)
    LoopVal =   %0 = phi i32* [ getelementptr inbounds ([4 x [6 x i32]], [4 x [6 x i32]]* @g_2168, i64 0, i64 3, i64 1), %for.end987 ], [ %g_1150.promoted, %lbl_2898 ]
  rewriteLoopExitValues: GEP rewrite bail out %0 != @g_2168
  opt: /repositories/llvm-project/llvm/lib/Transforms/Utils/LoopUtils.cpp:1354: int llvm::rewriteLoopExitValues(llvm::Loop *, llvm::LoopInfo *, llvm::TargetLibraryInfo *, llvm::ScalarEvolution *, const llvm::TargetTransformInfo *, llvm::SCEVExpander &, llvm::DominatorTree *, llvm::ReplaceExitVal, SmallVector<llvm::WeakTrackingVH, 16> &): Assertion `(!ZZAssert || Phi.ValidRewrite) && "Now that the SCEV is strict wrt pointer/integer types, this " "invariant is expected to be uphold by SCEV itself."' failed.

So we are trying to rewrite `%0 = phi`, and manage to rewrite it as the value when coming from `%for.end987` (and we must do so originally)
If we've come from `%lbl_2898`, then `%0` is stored into global, and loaded back.
Should that obscure pointer base as far as `isValidRewrite()` is concerned?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108043