Closed kpet closed 1 year ago
The core of this problem is that clspv is trying to perform multiple transforms on the same instruction. It identifies a few implicit casts, deals with the first and erases the basis of the second.
Here is a minimal testcase:
target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
target triple = "spir-unknown-unknown"
%struct.Line = type { i32, float, i32 }
define void @test(ptr addrspace(1) %lines, i32 %n) {
entry:
%gep1 = getelementptr inbounds %struct.Line, ptr addrspace(1) %lines, i32 %n
%gep2 = getelementptr inbounds i8, ptr addrspace(1) %gep1, i32 4
store float 0.000000e+00, ptr addrspace(1) %gep2, align 4
ret void
}
Run that source as test.ll
using the command:
clspv-opt test.ll --passes=simplify-pointer-bitcast
The transform is also iterating in a non-deterministic manner so I've seen a couple different failure modes on the original source. I plan to break up the implicit cast transform into a more pieces (with some redundancy) to avoid these issues.
Yes ImplicitCasts
should not be a DenseMap
.
As I split this up 3 tests have started to fail. I haven't debugged them all, but test6
in test/PointerCasts/opaque_trivial_casts.ll
seems to have been hiding a bug. Here is a snippet of that file (including test5
due to similarities):
; CHECK-LABEL: define void @test5(ptr %in) {
; CHECK: entry:
; CHECK: %0 = getelementptr i32, ptr %in, i32 2
; CHECK: ret void
; CHECK: }
define void @test5(ptr %in) {
entry:
%gep1 = getelementptr i32, ptr %in, i32 1
%gep2 = getelementptr i32, ptr %gep1, i32 1
ret void
}
; CHECK-LABEL: define void @test6(ptr %in) {
; CHECK: entry:
; CHECK: getelementptr float, ptr %in, i32 1
; CHECK-NEXT: ret void
; CHECK: }
define void @test6(ptr %in) {
entry:
%gep1 = getelementptr float, ptr %in, i32 1
%gep2 = getelementptr i32, ptr %gep1, i32 1
ret void
}
test5
seems correct in that it calculates an 8 byte offset from %in
, but test6
seems incorrect because it only calculates a 4 byte offset from %in
.
The bad test seems to stem from the pointer being in the private address space. The same test gives expected results if the pointer is in addrspace 1 for example.
All tests are passing after the refactor, but the minimal case for the original bug (see https://github.com/google/clspv/issues/1113#issuecomment-1551543610) appears to be transformed incorrectly.
The current result is:
target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
target triple = "spir-unknown-unknown"
%struct.Line = type { i32, float, i32 }
define void @test(ptr addrspace(1) %lines, i32 %n) {
entry:
%0 = getelementptr %struct.Line, ptr addrspace(1) %lines, i32 %n
store float 0.000000e+00, ptr addrspace(1) %0, align 4
ret void
}
The problem boils down to recalculating the GEP indices. The struct is 12 bytes, so the math used is to calculate the index is (1 / 12) which loses resolution.
When compiling the attached source, with the following options:
clspv crashes as follows (don't know why the assert is not firing, assertions are enabled):
source.txt