Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
| Time | Test | |
|---|---|---|
| 580 ms | LLVM.CodeGen/AMDGPU::Unknown Unit Message (" ") Script:
--
: 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/llc -march=amdgcn -mcpu=gfx600 -amdgpu-bypass-slow-div=0 -verify-machineinstrs < /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/sdiv64.ll | /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/FileCheck -check-prefix=GCN /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/sdiv64.ll
| |
| 710 ms | LLVM.CodeGen/AMDGPU::Unknown Unit Message (" ") Script:
--
: 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/llc -march=amdgcn -mcpu=gfx600 -amdgpu-bypass-slow-div=0 -verify-machineinstrs < /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/srem64.ll | /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/FileCheck -check-prefix=GCN /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/srem64.ll
| |
| 600 ms | LLVM.CodeGen/AMDGPU::Unknown Unit Message (" ") Script:
--
: 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/llc -march=amdgcn -mcpu=gfx600 -amdgpu-bypass-slow-div=0 -verify-machineinstrs < /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/udiv64.ll | /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/FileCheck -check-prefix=GCN /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/udiv64.ll
| |
| 520 ms | LLVM.CodeGen/AMDGPU::Unknown Unit Message (" ") Script:
--
: 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/llc -march=amdgcn -mcpu=gfx600 -amdgpu-bypass-slow-div=0 -verify-machineinstrs < /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/urem64.ll | /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/bin/FileCheck -check-prefix=GCN /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/llvm/test/CodeGen/AMDGPU/urem64.ll
| |
| 1,060 ms | LLVM.CodeGen/AMDGPU::Unknown Unit Message (" ") Script:
--
: 'RUN: at line 2'; c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\llc.exe -march=amdgcn -mcpu=gfx600 -amdgpu-bypass-slow-div=0 -verify-machineinstrs < C:\ws\workspace\amd64_windows_vs2017\llvm-project\llvm\test\CodeGen\AMDGPU\sdiv64.ll | c:\ws\workspace\amd64_windows_vs2017\llvm-project\build\bin\filecheck.exe -check-prefix=GCN C:\ws\workspace\amd64_windows_vs2017\llvm-project\llvm\test\CodeGen\AMDGPU\sdiv64.ll
| |
| View Full Test Results (12 Failed) |
Event Timeline
| mlir/lib/Conversion/GPUToSPIRV/ConvertGPUToSPIRV.cpp | ||
|---|---|---|
| 347 | The revision title says that it makes "spv.interface_var_abi attribute a required attribute", but this function seems to make it optional? | |
| mlir/lib/Conversion/GPUToSPIRV/ConvertGPUToSPIRV.cpp | ||
|---|---|---|
| 347 | This is more a escape hatch and is legacy. When I initially set this up it was convenient, but this is very limited in its usage. I would actually prefer to not have this, and any realistic user of the SPIR-V lowering will not be able to use this on many devices. So I can change the comment to make this clearer. | |
| mlir/lib/Conversion/GPUToSPIRV/ConvertGPUToSPIRV.cpp | ||
|---|---|---|
| 346 | Generates ... Returns false if any argument already has ABI specified. | |
| 356 | Let's put a comment here to explain why this if statement like: We need to wrap scalar values in a struct to satisfy Vulkan's interface variable requirements. We choose StorageBuffer for holding the struct here. Otherwise it might be a bit mysterious. | |
| mlir/test/Conversion/GPUToSPIRV/if.mlir | ||
| 19 | I guess this is changes made when this commit is truly breaking. But now we have a default behavior; we might want to revert these to simplify the tests. | |
| mlir/test/Conversion/GPUToSPIRV/simple.mlir | ||
| 34 | Instead of checking the default behavior, test the explicit behavior? Also, please make sure the assignment is not sequential as the default behavior. | |
| mlir/test/mlir-vulkan-runner/addf.mlir | ||
| 13 | I think we can omit the changes here; the in-tree Vulkan runner follows the sequential scheme. | |
Addressing comments and changing description to better reflect the
change post modifications.
| mlir/test/Conversion/GPUToSPIRV/if.mlir | ||
|---|---|---|
| 40 | Let's revert changes to these tests. They should pass as-is. Applies to following unaffected tests. | |
| mlir/test/Conversion/GPUToSPIRV/simple.mlir | ||
| 40 | I think we can just test the gpu.func itself without the wrapping gpu.module and the corresponding launch op? This way it's clearer regarding the intended use case of attaching these attributes: it gives us a way to use and control the CodeGen side as a stand-alone component. :) | |
Adressing comments
| mlir/lib/Conversion/GPUToSPIRV/ConvertGPUToSPIRV.cpp | ||
|---|---|---|
| 346 | Made it populates. | |
| mlir/test/Conversion/GPUToSPIRV/if.mlir | ||
| 40 | This is not really a change, the args are split, thats all. Will remove it anyway. | |
| mlir/test/Conversion/GPUToSPIRV/simple.mlir | ||
| 40 | Apparently the gpu.module is needed (gpu.func must have this as parent). Removed the funcOp that invokes the kernel though. | |
It would be nice to have a shorter oneliner for the first paragraph of the commit message and put the details in the second paragraph.
| mlir/test/Conversion/GPUToSPIRV/simple.mlir | ||
|---|---|---|
| 40 | Oh right. A recent change made gpu.func must appear in gpu.module | |