[LV] Fix for PR38110, LV encountered llvm_unreachable()

[hsaito]

Summary

truncateToMinimalBitWidths() doesn't handle all Instructions and the worst case is compiler crash via llvm_unreachable(). Fix is to add a case to handle PHINode and changed the worst case to NO-OP (from compiler crash).

[hsaito]

If I delete IBM target, the LIT test won't crash before the fix. If anyone can suggest an alternative so that I can move the lit test one level up, that would be great.

[sbaranga]

Seems reasonable to me.

Ideally we should be handling the phi node case (which is why the unreachable was there), but crashing doesn't seem like the right thing to do.

lib/Transforms/Vectorize/LoopVectorize.cpp

3279 ↗

(On Diff #155996)

Why do we need to have this if statement if we skip the transformation for unhanded instructions?

[hsaito]

In D49461#1166158, @sbaranga wrote:

Seems reasonable to me.

Ideally we should be handling the phi node case (which is why the unreachable was there), but crashing doesn't seem like the right thing to do.

Personally, I see a value in differentiating explicitly known handling versus just being conservative ----- but I don't mind converting that into a comment instead if that's preferred.

[sbaranga]

In D49461#1166719, @hsaito wrote:

In D49461#1166158, @sbaranga wrote:

Seems reasonable to me.

Ideally we should be handling the phi node case (which is why the unreachable was there), but crashing doesn't seem like the right thing to do.

Personally, I see a value in differentiating explicitly known handling versus just being conservative ----- but I don't mind converting that into a comment instead if that's preferred.

Sure. I don't have any strong preference here.

LGTM

[hsaito]

Just came back from vacation. Thanks. I take @sbaranga just forgot to set accept the revision and thus taking a liberty of doing so myself.

llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp

for (unsigned Part = 0; Part < UF; ++Part) {
         auto Elements0 = SI->getOperand(0)->getType()->getVectorNumElements();
         auto *O0 = B.CreateZExtOrTrunc(
             SI->getOperand(0), VectorType::get(ScalarTruncatedTy, Elements0));
         auto Elements1 = SI->getOperand(1)->getType()->getVectorNumElements();
         auto *O1 = B.CreateZExtOrTrunc(
             SI->getOperand(1), VectorType::get(ScalarTruncatedTy, Elements1));
 
         NewI = B.CreateShuffleVector(O0, O1, SI->getMask());
-      } else if (isa<LoadInst>(I)) {
+      } else if (isa<LoadInst>(I) || isa<PHINode>(I)) {
         // Don't do anything with the operands, just extend the result.
         continue;
       } else if (auto *IE = dyn_cast<InsertElementInst>(I)) {
         auto Elements = IE->getOperand(0)->getType()->getVectorNumElements();
         auto *O0 = B.CreateZExtOrTrunc(
             IE->getOperand(0), VectorType::get(ScalarTruncatedTy, Elements));
         auto *O1 = B.CreateZExtOrTrunc(IE->getOperand(1), ScalarTruncatedTy);
         NewI = B.CreateInsertElement(O0, O1, IE->getOperand(2));
       } else if (auto *EE = dyn_cast<ExtractElementInst>(I)) {
         auto Elements = EE->getOperand(0)->getType()->getVectorNumElements();
         auto *O0 = B.CreateZExtOrTrunc(
             EE->getOperand(0), VectorType::get(ScalarTruncatedTy, Elements));
         NewI = B.CreateExtractElement(O0, EE->getOperand(2));
       } else {
-        llvm_unreachable("Unhandled instruction type!");
+        // If we don't know what to do, be conservative and don't do anything.
+        continue;
       }
 
       // Lastly, extend the result.
       NewI->takeName(cast<Instruction>(I));
       Value *Res = B.CreateZExtOrTrunc(NewI, OriginalTy);
       I->replaceAllUsesWith(Res);
       cast<Instruction>(I)->eraseFromParent();
       Erased.insert(I);

llvm/trunk/test/Transforms/LoopVectorize/SystemZ/pr38110.ll

+; RUN: opt -passes='loop-vectorize' -mcpu=z13 -force-vector-width=2 -S < %s | FileCheck %s
+;
+; Forcing VF=2 to trigger vector code gen
+;
+; This is a test case to exercise more cases in truncateToMinimalBitWidths().
+; Test passes if vector code is generated w/o hitting llvm_unreachable().
+;
+; Performing minimal check in the output to ensure the loop is actually
+; vectorized.
+;
+; CHECK: vector.body
+
+target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
+target triple = "s390x-ibm-linux"
+
+define void @test(i32 zeroext %width, i8* nocapture %row, i16 zeroext %src, i16* nocapture readonly %dst) {
+entry:
+  %cmp10 = icmp eq i32 %width, 0
+  br i1 %cmp10, label %for.end, label %for.body.lr.ph
+
+for.body.lr.ph:                                   ; preds = %entry
+  %conv1 = zext i16 %src to i32
+  br label %for.body
+
+for.body:                                         ; preds = %for.inc, %for.body.lr.ph
+  %i.012 = phi i32 [ 0, %for.body.lr.ph ], [ %inc, %for.inc ]
+  %sp.011 = phi i8* [ %row, %for.body.lr.ph ], [ %incdec.ptr, %for.inc ]
+  %0 = load i8, i8* %sp.011, align 1
+  %conv = zext i8 %0 to i32
+  %cmp2 = icmp eq i32 %conv, %conv1
+  br i1 %cmp2, label %if.then, label %for.inc
+
+if.then:                                          ; preds = %for.body
+  %1 = load i16, i16* %dst, align 2
+  %conv4 = trunc i16 %1 to i8
+  store i8 %conv4, i8* %sp.011, align 1
+  br label %for.inc
+
+for.inc:                                          ; preds = %for.body, %if.then
+  %inc = add nuw i32 %i.012, 1
+  %incdec.ptr = getelementptr inbounds i8, i8* %sp.011, i64 1
+  %exitcond = icmp eq i32 %inc, %width
+  br i1 %exitcond, label %for.end.loopexit, label %for.body
+
+for.end.loopexit:                                 ; preds = %for.inc
+  br label %for.end
+
+for.end:                                          ; preds = %for.end.loopexit, %entry
+  ret void
+}