[PATCH] D69490: [LoopIdiomRecognize] Avoid trying to create a pattern from the address of a thread local.

[Ben via Phabricator via llvm-commits]

bobsayshilol created this revision.
bobsayshilol added a reviewer: lebedev.ri.
Herald added subscribers: llvm-commits, dexonsmith, mehdi_amini.
Herald added a project: LLVM.

The address of a thread local is technically constant however its value isn't known until the thread of execution starts, so constructing a GlobalVariable containing its address doesn't make sense unless it is also thread local and initialised at thread startup. LLVM doesn't appear to have a concept of TLS init like it does for static construction and destruction (ie something similar to `llvm::appendToGlobal{C,D}tors()` but for TLS) and clang's TLS init code isn't available down in LLVM, so this patch takes the easy option and avoids the issue altogether by rejecting values that are thread locals.

A better demonstration of where this code breaks is with:

  #include <cstdio>
  
  thread_local int global_var;
  
  __attribute__((noinline))
  void test_func(int*(&ptrs)[2])
  {
    printf("&global_var = %p\n", &global_var);
    for (auto& p : ptrs)
      p = &global_var;
  }
  
  int main()
  {
    int*test[2]{};
    test_func(test);
    printf("test[0] = %p\n", test[0]);
  }

which, on platforms that support `memset_pattern16()` and with optimisations enabled, will print something like:

  &global_var = 0x7f1e51f77f3c
  test[0] = 0x403de0

where any reads/writes of `*test[0]` would indeed crash at the mysterious address `0x403de0`.


Repository:
  rL LLVM

https://reviews.llvm.org/D69490

Files:
  lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  test/Transforms/LoopIdiom/thread-local.ll


Index: test/Transforms/LoopIdiom/thread-local.ll
===================================================================
--- test/Transforms/LoopIdiom/thread-local.ll
+++ test/Transforms/LoopIdiom/thread-local.ll
@@ -0,0 +1,29 @@
+; RUN: opt -loop-idiom < %s -S | FileCheck %s
+
+target triple = "x86_64-apple-darwin10.0.0"
+
+;void test_tls(int **ptrs) {
+;  thread_local int tls_var;
+;  for (int i = 0; i < 1000; i++) {
+;    p[i] = &tls_var;
+;  }
+;}
+ at tls_var = internal thread_local global i32 0, align 4
+
+define void @test_tls(i32** nocapture %ptrs) nounwind ssp {
+entry:
+  br label %for.body
+
+for.body:                                         ; preds = %entry, %for.body
+  %indvar = phi i64 [ 0, %entry ], [ %indvar.next, %for.body ]
+  %arrayidx = getelementptr i32*, i32** %ptrs, i64 %indvar
+  store i32* @tls_var, i32** %arrayidx, align 8
+  %indvar.next = add i64 %indvar, 1
+  %exitcond = icmp eq i64 %indvar.next, 1000
+  br i1 %exitcond, label %for.end, label %for.body
+
+for.end:                                          ; preds = %for.body
+  ret void
+; CHECK-LABEL: @test_tls(
+; CHECK-NOT: call void @memset_pattern
+}
Index: lib/Transforms/Scalar/LoopIdiomRecognize.cpp
===================================================================
--- lib/Transforms/Scalar/LoopIdiomRecognize.cpp
+++ lib/Transforms/Scalar/LoopIdiomRecognize.cpp
@@ -441,6 +441,13 @@
   if (!C)
     return nullptr;
 
+  // While technically constant, the address of a thread_local GlobalVariable
+  // isn't known until runtime and would require additional tls init code to
+  // handle correctly.
+  if (GlobalVariable *GV = dyn_cast<GlobalVariable>(C))
+    if (GV->isThreadDependent())
+      return nullptr;
+
   // Only handle simple values that are a power of two bytes in size.
   uint64_t Size = DL->getTypeSizeInBits(V->getType());
   if (Size == 0 || (Size & 7) || (Size & (Size - 1)))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69490.226593.patch
Type: text/x-patch
Size: 1897 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191027/6a7d19f0/attachment.bin>