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

[LiDongjin via Phabricator via llvm-commits]

LiDongjin added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6562
   if (match(RetI, m_ZExtOrSExt(m_Value()))) {
-    if (!RetI->hasOneUser())
+    if (!match(RetI, m_OneUse(m_Value())))
       return None;
----------------
dmgreen wrote:
> This is very deliberately OneUser, not OneUse. It needs to match `mul(zext(x), zext(x))`
Yes, That's my mistakes which I misunderstood the code here.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6567
+
+  if (match(RetI, m_Mul(m_Value(), m_Value()))) {
+    if (!match(RetI, m_OneUse(m_Value())))
----------------
dmgreen wrote:
> Mul is probably OK with OneUse. Can it use `if (match(RetI, m_OneUse(m_Mul(m_Value(), m_Value())))) {`
Dose It seems likes:
```
if (match(RetI, m_OneUse(m_Mul(m_Value(), m_Value())))) {
  if (RetI->user_back()->getOpcode() == Instruction::Add)
    RetI = RetI->user_back();
}
if(match(RetI, m_Mul(m_Value(), m_Value())) && !match(RetI, m_OneUse(m_Value()))) {
  return None;
}
   
```


================
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:
> 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


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

https://reviews.llvm.org/D136227