[PATCH] D146872: [1/11][IR] Permit load/store/alloca for struct of the same scalable vector type

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/docs/LangRef.rst:750
+loads, stores, allocas, or GEPs. The only exception to this rule is for structs
+that contains scalable vectors of the same type (e.g. ``{<vscale x 2 x i32>,
+<vscale x 2 x i32>}`` contains the same type while ``{<vscale x 2 x i32>,
----------------
contains -> contain


================
Comment at: llvm/docs/LangRef.rst:752
+<vscale x 2 x i32>}`` contains the same type while ``{<vscale x 2 x i32>,
+<vscale x 2 x i64>}`` doesn't). These kind of structs (we may call them
+homogeneous scalable vector structs) are considered sized and can be used in
----------------
kind -> kinds


================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:223
+    SCDB_ContainsScalableVector = 16,
+    SCDB_NotContainScalableVector = 32
   };
----------------
Contain -> Contains


================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:294
+  /// Note that the definition of homogeneous scalable vector type is not
+  /// recursive here. That means the following structure will return a false
+  /// when calling this function.
----------------
a false -> false


================
Comment at: llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:155
 
-          // Scalable vectors may need a special StackID to distinguish
-          // them from other (fixed size) stack objects.
-          if (isa<ScalableVectorType>(Ty))
+          // Scalable vectors and structure that contains scalable vector may
+          // need a special StackID to distinguish them from other (fixed size)
----------------
structure -> structs
vector -> vectors


================
Comment at: llvm/lib/IR/Type.cpp:626
+  // The ONLY special case of opaque types inside a Struct that is considered
+  // sized is when the elements are homogeneous of a scalable vector type.
+  if (containsHomogeneousScalableVectorTypes()) {
----------------
This has no relation to opaque types.


================
Comment at: llvm/test/Other/load-scalable-vector-struct.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt -passes=verify -S < %s 2>&1 | FileCheck %s
+
----------------
Move these two tests into llvm/test/Assembler and combine them.


================
Comment at: llvm/test/Transforms/InstCombine/scalable-vector-struct-extractvalue.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt -passes=instcombine -S < %s 2>&1 | FileCheck %s
+
----------------
You've added three bailouts in InstCombine -- does this test case cover all three of them?

Also, don't we need a test case for the check in SROA as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146872