llvmbot
commented
1 year ago @llvm/issue-subscribers-backend-amdgpu
Open HazyFish opened 1 year ago
llvmbot
commented
1 year ago @llvm/issue-subscribers-backend-amdgpu
gandhi56
commented
1 year ago Reproduced on r600 with unhandled address space error in debug mode, and on amdgcn with the cannot select store instruction error in SelectionDAG in debug mode.
Compiled successfully on X86.
gandhi56
commented
1 year ago Following code compiles successfully, with SelectionDAG and GlobalISel:
; RUN: llc -mtriple=amdgcn %s -o - | FileCheck %s
; CHECK: f
define void @f() {
BB:
br label %BB1
BB1: ; preds = %BB1, %BB
%A1 = alloca <32 x i64>
%S = shufflevector <32 x i64> zeroinitializer, <32 x i64> <i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1>, <32 x i32> undef
store <32 x i64> %S, ptr %A1
br i1 false, label %BB1, label %BB2
BB2: ; preds = %BB5, %BB2, %BB1
%A = alloca i1
%L = load i1, ptr %A
%B1 = add i1 %L, %L
%B = ashr i1 false, true
%B3 = or i1 %B, %B1
%G1 = getelementptr i1, ptr %A, i1 %B
store i1 %B3, ptr %G1
br i1 %B, label %BB2, label %BB4
BB4: ; preds = %BB4, %BB2
%G = getelementptr i1, ptr %A, i1 %B
%L1 = load i1, ptr %G
%L2 = load i1, ptr %G
%B4 = add i1 %L2, true
%C1 = icmp sgt i1 %B4, %L2
; store i1 %C1, ptr %G <-- Commenting this store instruction
%B2 = ashr i1 %B4, %L2
%B5 = xor i1 %B2, %L1
%C2 = icmp sgt i1 %C1, %B5
%C4 = icmp eq i1 %C1, %L2
%C3 = icmp ult i1 %C2, %C4
br i1 %C3, label %BB4, label %BB5
BB5: ; preds = %BB4
store i1 %B, ptr %G
%C = icmp sgt i64 0, -1
br i1 %C, label %BB2, label %BB3
BB3: ; preds = %BB5
ret void
}
arsenm
commented
1 year ago The issue is for r600 not amdgcn
gandhi56
commented
1 year ago gandhi56
commented
1 year ago For the R600 case, I assume it's okay to return 128 for the vectorizing register bit width. That renders code with an infinite loop. Is there a reason why AMDGPUUnifyDivergentExitNodes is not scheduled for R600?
arsenm
commented
1 year ago For the R600 case, I assume it's okay to return 128 for the vectorizing register bit width. That renders code with an infinite loop. Is there a reason why AMDGPUUnifyDivergentExitNodes is not scheduled for R600?
Because it's barely (not really) maintained. Don't really follow the point about the infinite loop. The vector register bit width shouldn't matter for producing functioning code
gandhi56
commented
1 year ago https://godbolt.org/z/3hjjfsThn
The instruction %B = ashr i1 false, true propagates poison values. These poison values stick around before instruction selector, which selectionDAG fails to to select. I don't think there is anything to be done for this case as the input code is weird to begin with. Thoughts?
arsenm
commented
1 year ago https://godbolt.org/z/3hjjfsThn
The instruction
%B = ashr i1 false, truepropagates poison values. These poison values stick around before instruction selector, which selectionDAG fails to to select. I don't think there is anything to be done for this case as the input code is weird to begin with. Thoughts?
poison doesn't mean "won't compile" and nothing is weird with that, that part should just work. The flat pointers aren't going to work on r600. Also, the alloca is the wrong address space. The only thing worth fixing here would be the " no registers from class available" if it manifests using address spaces that are supposed to work.
With the address spaces fixed, this is broken for other reasons:
target triple = "r600--"
define void @f() {
BB:
br label %BB1
BB1: ; preds = %BB1, %BB
%A1 = alloca <32 x i64>, addrspace(5)
%S = shufflevector <32 x i64> zeroinitializer, <32 x i64> <i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1, i64 -1>, <32 x i32> undef
store <32 x i64> %S, ptr addrspace(5) %A1
br i1 false, label %BB1, label %BB2
BB2: ; preds = %BB5, %BB2, %BB1
%A = alloca i1, addrspace(5)
%L = load i1, ptr addrspace(5) %A
%B1 = add i1 %L, %L
%B = ashr i1 false, true
%B3 = or i1 %B, %B1
%G1 = getelementptr i1, ptr addrspace(5) %A, i1 %B
store i1 %B3, ptr addrspace(5) %G1
br i1 %B, label %BB2, label %BB4
BB4: ; preds = %BB4, %BB2
%G = getelementptr i1, ptr addrspace(5) %A, i1 %B
%L1 = load i1, ptr addrspace(5) %G
%L2 = load i1, ptr addrspace(5) %G
%B4 = add i1 %L2, true
%C1 = icmp sgt i1 %B4, %L2
store i1 %C1, ptr addrspace(5) %G
%B2 = ashr i1 %B4, %L2
%B5 = xor i1 %B2, %L1
%C2 = icmp sgt i1 %C1, %B5
%C4 = icmp eq i1 %C1, %L2
%C3 = icmp ult i1 %C2, %C4
br i1 %C3, label %BB4, label %BB5
BB5: ; preds = %BB4
store i1 %B, ptr addrspace(5) %G
%C = icmp sgt i64 0, -1
br i1 %C, label %BB2, label %BB3
BB3: ; preds = %BB5
ret void
}
; Assertion failed: (!ExitingMBBs.empty() && "Infinite Loop not supported"), function mergeLoop, file R600MachineCFGStructurizer.cpp, line 1010.
Overall this is just demonstrating that r600 was never really completed and barely works.
lorn10
commented
11 months ago Overall this is just demonstrating that r600 was never really completed and barely works.
Sorry for jumping in. Short question, what means that statement for end-users especially in regard to OpenCL? Could this be interpreted as one of the reasons why clover had much more difficulties on TeraScale compared to GCN hardware?
I think it can be said that at least in the past clover and (older) LLVM worked to some degree also on TeraScale hardware even if clpeak had problems since literately ever. And it looks that despite those deficits in LLVM the r600 backend could be used for even more interesting stuff like this one here: clover: Add LLVM bitcode as IL type.
Perhaps it would make sense to mention the "r600 LLVM limitations and overall incompleteness" at a more official place like in the LLVM AMDGPU backend user guide. :thinking:
And finally, it looks that r600 support in LLVM could be dropped at some point, - simply because there is no more practical use for it. Regarding OpenCL in Mesa there will exist also for TeraScale hardware rusticl as an alternative for "LLVM clover" even if it is somewhat slower than clover. Yeah, and before r600 will be purged please add your "AMDGPU/R600: Special case addrspacecast lowering for null" commit. I hoped it would land in LLVM 17. :wink:
/cc Maybe something from here is of interest for @Triang3l
gandhi56
commented
11 months ago I tried inserting the AMDGPUUnifyDivergentExitNodes pass to ensure there is an exit block in CFGs that don't already have one due to infinite loops. That does not suffice and that more work is required in the R600MachineCFGStructurizer. Is it worth the effort to fix this issue for an architecture that may hardly be used these days?
lorn10
commented
11 months ago @gandhi56 Yes, you are most likely right. As far I know the r600 LLVM backend is nowadays rarely used. I am aware regarding Linux only for Mesa clover and probably (partially) also the r600 Radeon Mesa driver. But perhaps some other AMD related stuff is still depending on it? I have no idea. Someone from AMD should bring here more clarity.
And finally, a use of the r600 LLVM backend in the potential TeraScale Vulkan project "Terakan" is because of the above statement highly unlikely. So unlike to RADV Terakan will have almost sure only a NIR>SFN and no NIR>LLVM path. :thinking:
Description
The following code crashes
llcwith "LLVM ERROR: no registers from class available" during "Greedy Register Allocator" pass for release build and "unhandled address space" during "GPU Load and Store Vectorizer" for debug build when targeting R600.The code also crashes
llcwith "LLVM ERROR: Cannot select: t13: ch = store<(store (s8) intoi1* poison), trunc to i8> t0, t15, undef:i64, undef:i64" when targeting AMDGCN, but compiles successfully for other architectures.Reproduction
https://godbolt.org/z/5rv3f6r8r
Code
Stack Trace
Release Build targeting R600
Debug Build targeting R600
Debug Build targeting AMDGCN