[PATCH] D136227: [LoopVectorize] Fix crash on "Cannot dereference end iterator!"(PR56627)

[LiDongjin via Phabricator via llvm-commits]

LiDongjin added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/pr56627.ll:1
+; RUN: opt < %s -S -passes=loop-vectorize -force-vector-width=2 | FileCheck %s
+
----------------
dmgreen wrote:
> LiDongjin wrote:
> > dmgreen wrote:
> > > I think you need to remove -force-vector-width=2 to reproduce the original issue.
> > Yes, I have some confuse about this test. 
> > For the original issue, It needs to produce the reduce instruction to trigger the crash bug. So it seems that we do not need to test every line in the final vectorize program which we just check program can run without crash.
> > 
> > And if i remove -force-vector-width=2, Is there some future changes that disables or changes the vectorize of this program ? (like enable vector-width=4 or not produce vectorize program)
> > So Can i just use the simple test like below:
> > 
> > ```
> > ; RUN: opt < %s -S -passes=loop-vectorize | FileCheck %s
> > ; Check that we can vectorize this loop without crashing.
> > 
> > target triple = "aarch64-none-linux-gnu"
> > define void @quux() {
> > ; CHECK-LABEL: @quux(
> > ```
> > But it seems the test can not enough to trrigle bug that check we produce reduce instr
> >  
> > ```
> > ; CHECK: call float @llvm.vector.reduce.fadd.v2f32
> > ```
> > So i think this check is necessary, and use -force-vector-width=2 to ensure correct with any changes
> This example doesn't seem to trigger the crash: https://llvm.godbolt.org/z/czfz7qsoq
> It does if the -force-vector-width=2 is removed: https://llvm.godbolt.org/z/cqMzTTvj1
> So from that, the argument needs to be removed. Are you seeing something different?
> 
> I think that other than that, the test seems OK to me. The auto-generated check lines are OK to keep, and the comment explains that we are really protecting from a crash. The check lines could be reduced, but the auto-generated check lines are pretty standard-practice.
I remove the -force-vector-width=2  to keep the testcase same with origin.
But the testcase will not vectorize,So i just check the func label for now.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136227