[PATCH] D87538: [VectorCombine] Don't vectorize scalar load under tsan

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: lebedev.ri, RKSimon, spatel, xbolva00.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
MaskRay requested review of this revision.

The D81766 <https://reviews.llvm.org/D81766> optimization should be suppressed under tsan due to potential
spurious data race reports:

  struct A {
    int i;
    const short s; // scalar load on s should not be vectorized
    int modify;    // being concurrently modified
    long pad1, pad2;
  };


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87538

Files:
  llvm/lib/Transforms/Vectorize/VectorCombine.cpp
  llvm/test/Transforms/VectorCombine/X86/load.ll


Index: llvm/test/Transforms/VectorCombine/X86/load.ll
===================================================================
--- llvm/test/Transforms/VectorCombine/X86/load.ll
+++ llvm/test/Transforms/VectorCombine/X86/load.ll
@@ -292,6 +292,22 @@
   ret <8 x i16> %r
 }
 
+; Negative test - disable under tsan because extended load may overlap bytes
+; being concurrently modified.
+
+define <8 x i16> @gep10_load_i16_insert_v8i16_tsan(<8 x i16>* align 16 dereferenceable(32) %p) sanitize_thread {
+; CHECK-LABEL: @gep10_load_i16_insert_v8i16_tsan(
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds <8 x i16>, <8 x i16>* [[P:%.*]], i64 1, i64 0
+; CHECK-NEXT:    [[S:%.*]] = load i16, i16* [[GEP]], align 16
+; CHECK-NEXT:    [[R:%.*]] = insertelement <8 x i16> undef, i16 [[S]], i64 0
+; CHECK-NEXT:    ret <8 x i16> [[R]]
+;
+  %gep = getelementptr inbounds <8 x i16>, <8 x i16>* %p, i64 1, i64 0
+  %s = load i16, i16* %gep, align 16
+  %r = insertelement <8 x i16> undef, i16 %s, i64 0
+  ret <8 x i16> %r
+}
+
 ; Negative test - can't safely load the offset vector, but could load+shuffle.
 
 define <8 x i16> @gep10_load_i16_insert_v8i16_deref(<8 x i16>* align 16 dereferenceable(31) %p) {
Index: llvm/lib/Transforms/Vectorize/VectorCombine.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -688,6 +688,7 @@
     return false;
 
   bool MadeChange = false;
+  bool IsTsan = F.hasFnAttribute(Attribute::SanitizeThread);
   for (BasicBlock &BB : F) {
     // Ignore unreachable basic blocks.
     if (!DT.isReachableFromEntry(&BB))
@@ -700,7 +701,10 @@
       if (isa<DbgInfoIntrinsic>(I))
         continue;
       Builder.SetInsertPoint(&I);
-      MadeChange |= vectorizeLoadInsert(I);
+      // Do not vectorize scalar load under tsan. The extended load may overlap
+      // bytes being concurrently modified and cause spurious data races.
+      if (!IsTsan)
+        MadeChange |= vectorizeLoadInsert(I);
       MadeChange |= foldExtractExtract(I);
       MadeChange |= foldBitcastShuf(I);
       MadeChange |= scalarizeBinopOrCmp(I);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87538.291303.patch
Type: text/x-patch
Size: 2178 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200911/463c61cb/attachment.bin>