[PATCH] D153316: [AArch64][SelectionDAG] fix infinite loop caused by legalizing & combining CONCAT_VECTORS

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Can you upload with context? https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface. It makes the reviews easier to read.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:23171
+    //
+    // https://github.com/llvm/llvm-project/issues/63322
+    //
----------------
I'm not sure this comment adds a lot to the existing code. It might make a better summary for the patch.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:23182
+    if (In.getOpcode() == ISD::CONCAT_VECTORS && In.hasOneUse() &&
+        (!In.getValueType().isScalableVector() || !LegalDAG)) {
       unsigned NumOps = N->getNumOperands() * In.getNumOperands();
----------------
Does this need to check for isScalableVector, or can it just check for !LegalDAG? Or does that alter other tests?


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-sve-concat_vectors.ll:1
+; RUN: llc -O3 -mcpu=neoverse-n2 < %s | FileCheck %s
+; CHECK: @allocno_reload_assign
----------------
This likely needs a -mtriple=aarch64-none-eabi, in order to work when the default target is not aarch64.
And is it possible to use -mattr=+sve2 instead of a specific cpu?


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-sve-concat_vectors.ll:4
+
+; Function Attrs: nocallback nofree nosync nounwind willreturn writeonly
+declare void @llvm.masked.scatter.nxv16i8.nxv16p0(<vscale x 16 x i8>, <vscale x 16 x ptr>, i32 immarg, <vscale x 16 x i1>) #0
----------------
I usually remove `; Function Attrs:..`


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-sve-concat_vectors.ll:18
+
+attributes #0 = { nocallback nofree nosync nounwind willreturn writeonly }
----------------
Can this be removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153316