[PATCH] D39295: [LSV] Skip all non-byte sizes, not only less than eight bits

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope created this revision.

The code comments indicate that no effort has been spent on
handling load/stores when the size isn't a multiple of the
byte size correctly. However, the code only avoided types
smaller than 8 bits. So for example a load of an i28 could
still be considered as a candidate for vectorization.

This patch adjusts the code to behave according to the code
comment.

The test case used to hit the following assert when
trying to use "cast" an i32 to i28 using CreateBitOrPointerCast:

opt: ../lib/IR/Instructions.cpp:2565: Assertion `castIsValid(op, S, Ty) && "Invalid cast!"' failed.
#0 PrintStackTraceSignalHandler(void*)
#1 SignalHandler(int)
#2 __restore_rt
#3 __GI_raise
#4 __GI_abort
#5 __GI___assert_fail
#6 llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, llvm::Twine const&, llvm::Instruction*)
#7 llvm::IRBuilder<llvm::ConstantFolder, llvm::IRBuilderDefaultInserter>::CreateBitOrPointerCast(llvm::Value*, llvm::Type*, llvm::Twine const&)
#8 (anonymous namespace)::Vectorizer::vectorizeLoadChain(llvm::ArrayRef<llvm::Instruction*>, llvm::SmallPtrSet<llvm::Instruction*, 16u>*)


https://reviews.llvm.org/D39295

Files:
  include/llvm/IR/DataLayout.h
  lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
  test/Transforms/LoadStoreVectorizer/X86/non-byte-size.ll


Index: test/Transforms/LoadStoreVectorizer/X86/non-byte-size.ll
===================================================================
--- /dev/null
+++ test/Transforms/LoadStoreVectorizer/X86/non-byte-size.ll
@@ -0,0 +1,30 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -load-store-vectorizer -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+
+%rec = type { i32, i28 }
+
+; We currently do not optimize this scenario.
+; But we verify that we no longer crash when compiling this.
+define void @test1(%rec* %out, %rec* %in) {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:    [[IN1:%.*]] = getelementptr [[REC:%.*]], %rec* [[IN:%.*]], i16 0, i32 0
+; CHECK-NEXT:    [[IN2:%.*]] = getelementptr [[REC]], %rec* [[IN]], i16 0, i32 1
+; CHECK-NEXT:    [[VAL1:%.*]] = load i32, i32* [[IN1]], align 8
+; CHECK-NEXT:    [[VAL2:%.*]] = load i28, i28* [[IN2]]
+; CHECK-NEXT:    [[OUT1:%.*]] = getelementptr [[REC]], %rec* [[OUT:%.*]], i16 0, i32 0
+; CHECK-NEXT:    [[OUT2:%.*]] = getelementptr [[REC]], %rec* [[OUT]], i16 0, i32 1
+; CHECK-NEXT:    store i32 [[VAL1]], i32* [[OUT1]], align 8
+; CHECK-NEXT:    store i28 [[VAL2]], i28* [[OUT2]]
+; CHECK-NEXT:    ret void
+;
+  %in1 = getelementptr %rec, %rec* %in, i16 0, i32 0
+  %in2 = getelementptr %rec, %rec* %in, i16 0, i32 1
+  %val1 = load i32, i32* %in1, align 8
+  %val2 = load i28, i28* %in2
+  %out1 = getelementptr %rec, %rec* %out, i16 0, i32 0
+  %out2 = getelementptr %rec, %rec* %out, i16 0, i32 1
+  store i32 %val1, i32* %out1, align 8
+  store i28 %val2, i28* %out2
+  ret void
+}
+
Index: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
===================================================================
--- lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -612,13 +612,13 @@
 
       // Skip weird non-byte sizes. They probably aren't worth the effort of
       // handling correctly.
-      unsigned TySize = DL.getTypeSizeInBits(Ty);
-      if (TySize < 8)
+      if (!DL.isByteSized(Ty))
         continue;
 
       Value *Ptr = LI->getPointerOperand();
       unsigned AS = Ptr->getType()->getPointerAddressSpace();
       unsigned VecRegSize = TTI.getLoadStoreVecRegBitWidth(AS);
+      unsigned TySize = DL.getTypeSizeInBits(Ty);
 
       // No point in looking at these if they're too big to vectorize.
       if (TySize > VecRegSize / 2)
@@ -648,13 +648,15 @@
 
       // Skip weird non-byte sizes. They probably aren't worth the effort of
       // handling correctly.
-      unsigned TySize = DL.getTypeSizeInBits(Ty);
-      if (TySize < 8)
+      if (!DL.isByteSized(Ty))
         continue;
 
       Value *Ptr = SI->getPointerOperand();
       unsigned AS = Ptr->getType()->getPointerAddressSpace();
       unsigned VecRegSize = TTI.getLoadStoreVecRegBitWidth(AS);
+      unsigned TySize = DL.getTypeSizeInBits(Ty);
+
+      // No point in looking at these if they're too big to vectorize.
       if (TySize > VecRegSize / 2)
         continue;
 
Index: include/llvm/IR/DataLayout.h
===================================================================
--- include/llvm/IR/DataLayout.h
+++ include/llvm/IR/DataLayout.h
@@ -375,6 +375,15 @@
   /// [*] The alloc size depends on the alignment, and thus on the target.
   ///     These values are for x86-32 linux.
 
+  /// \brief Returns true if the number of bits necessary to hold the specified
+  /// type is a multiple of the byte size.
+  ///
+  /// The type passed must have a size (Type::isSized() must return true).
+  bool isByteSized(Type *Ty) const {
+    uint64_t TySize = getTypeSizeInBits(Ty);
+    return (TySize % 8) == 0;
+  }
+
   /// \brief Returns the number of bits necessary to hold the specified type.
   ///
   /// For example, returns 36 for i36 and 80 for x86_fp80. The type passed must


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39295.120270.patch
Type: text/x-patch
Size: 3835 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171025/2c48e9f3/attachment.bin>