JonChesterfield added a comment.

We'll have slightly indirect testing once this is used to enable D99656 
<https://reviews.llvm.org/D99656>. There are two pieces that can be tested:

1/ The clang handling. That we can test with a minor change. Add a command line 
argument to clang that specifies the name of the tool to call to get the 
architecture (and defaults to amdgpu-arch). Then write N tests that call N 
different shell scripts that return the various cases. The plumbing for that 
argument may prove useful when adding a corresponding nvptx toool.

2/ The call into hsa. That probably means stubbing out hsa. Kind of interested 
in that - a hsa library that can be jury rigged to fail various calls is a path 
to a robust amdgpu plugin. Way more complexity in that fake stub than in this 
code though. I don't think that's worthwhile here.



================
Comment at: clang/tools/amdgpu-arch/AMDGPUArch.cpp:42
+  if (Status != HSA_STATUS_SUCCESS) {
+    fprintf(stderr, "Unable to initialize HSA\n");
+  }
----------------
missed one, return 1 on failure to initialize


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99949

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to