[PATCH] D98608: [Attributor] Make sure Heap2Stack works properly on a GPU target

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:961
+  /// Return true if the stack (llvm::Alloca) can be accessed by other threads.
+  bool stackIsAccessibleByOtherThreads() {
+    return !targetIsGPU();
----------------
I don't think it is limited to just GPUs. Generally, it is applicable to machines where stack is not shared by multiple threads e.g. this is applicable to X86 threading model too if I am not mistaken - stack is indeed private.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:5172
+    // doesn't apply as the pointer could be shared and needs to be places in
+    // "sharable" memory.
+    if (!StackIsAccessibleByOtherThreads) {
----------------
Typo in "sharable"; "needs to be places" ->placed.


================
Comment at: llvm/test/Transforms/Attributor/heap_to_stack_gpu.ll:7
 
-declare noalias i8* @malloc(i64)
+target triple = "nvptx64"
+
----------------
May be have a test "amdgpu" triple too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98608