[PATCH] D98351: [llvm-opt] Bug fix within combining FP vectors

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added a comment.

Hey Nash,

I also added some comments. 
I hope it is ok.

Carol



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1597
   // Make a vector type from the minimal type.
-  return FixedVectorType::get(MinType, NumElts);
+  return VectorType::get(MinType, NumElts, isa<ScalableVectorType>(CVVTy));
 }
----------------
Just in case, I don't think that you should cast this to be ScalableVectorType.
I believe you can leave the return as it is originally
Especially if Sander is saying to have an earlier return when doing the for.


================
Comment at: llvm/test/Transforms/InstCombine/instcombine-vectors.ll:2
+; RUN: opt -instcombine -march=armv8.1-a+sve -S -o - < %s | FileCheck %s
+
+define <vscale x 2 x float> @foo(<vscale x 2 x double> %a) {
----------------
Usually we also add this line in our tests:
; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t

; If this check fails please read test/CodeGen/AArch64/README for instructions on how to resolve it.$
; WARN-NOT: warning

And I believe this test should be in inside test/Transforms/InstCombine/AArch64


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98351