[libc-commits] [PATCH] D149517: [libc] Enable running libc unit tests on AMDGPU

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/config/gpu/entrypoints.txt:71
-    libc.src.stdlib.strtoul
-    libc.src.stdlib.strtoull
 
----------------
You can keep the entrypoints but skip the tests with a detailed comment. Up to you.


================
Comment at: libc/test/src/__support/File/CMakeLists.txt:4
+  # we just skip everything about files. The GPU has a mutex implementation but
+  # it is simply passthrough and can't be used.
   return()
----------------
May the actual reason is that we don't yet support files on the GPU? If yes, then may be just say that?


================
Comment at: libc/test/src/__support/blockstore_test.cpp:95
+
+// FIXME: The AMDGPU backend hangs if these function calls are combined.
+TEST_F(LlvmLibcBlockStoreTest, BackReverse) { back_test<true>(); }
----------------
Without the context of this patch, the word "these" becomes vague. Can you may be say:

```
Combing this test with the above test makes the AMDGPU backend
generate code which hangs.
TODO: Investigate why the AMDGPU backend is generating such code.
```


================
Comment at: libc/test/src/stdio/CMakeLists.txt:6
+  # entrypoints.
+  return()
+endif()
----------------
Those tests which depend on entrypoints will not be included. I think you want to `return()` on line 326?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149517