[NFC] Add testing for cross-compilation with SPIR-V backend#3564
[NFC] Add testing for cross-compilation with SPIR-V backend#3564MrSidims merged 5 commits intoKhronosGroup:mainfrom
Conversation
b02ebc9 to
d82c774
Compare
Oops, need to update email address in git settings on my machine. |
|
I love the idea, I'm just a bit confused with the intent. Are we planning to merge this as is, or is this kind of an RFC? Merging this patch would break CI, wouldn't it? Or are we planning to fix every error first? Also, wondering if it could be possible to add this under an option, so only those interested run cross testing. |
"It works on my machine" ┐('~` )┌". At least with manually changed LLVM major version. But currently it breaks CI as due to not found llc, so I need to tweak the scripts a bit.
It's not gating the merge IMHO. But I do plan to at least investigate some of them. For example, there are image tests failing with assertion during reverse when compiled with llc. It's actually SPIR-V backend bug as it doesn't have
My personal preference to enable these tests by default in night and in pre-commit. SPIR-V Writer should be replaced with SPIR-V backend by default in many compilation flows. SPIR-V Reader will live longer as there is no alternative currently. Therefore we should prepare existing CI to test both. Oh, yeah, and probably I should offload this:
to AI and get, analyze and amend the results into the PR before merging (it's unlikely that I will have enough passion to insert some stub functions in each test to use some previously-DCEed results and update checks manually). |
Okay, the CI crash made me panic a bit thinking that merging this would mean living with broken CI. If this doesn't break CI I'm fine with follow up PRs to fix the existing failures.
We could still enable the option by default in CI, I'm just thinking that maybe some people doesn't care about SPIRV BE at all, but I don't have strong feelings on this. If it's too complicated or requires too many changes, I'm fine with things as they are. |
No extensions/ nor DebugInfo/ tests are touched. For tests that are failing appropriate TODO and FIXME are added. Variants of errors: 1. llc crash run on the test; 2. llvm-spirv -r crash when reverse translating llc-generated SPIR-V; 3. FileCheck missmatches that are not yet investigated as a part of this patch. Note, few IR samples in llvm-spirv LITs are producing empty SPIR-V modules when compiled with SPIR-V backend due to DCE. This has to be fixed.
d82c774 to
14df3cd
Compare
|
@maarquitos14 @vmaksimo I believe it's ready, changes from the last patch: added llc detection to lit cfg and llc with SPIR-V backend build in CI |
maarquitos14
left a comment
There was a problem hiding this comment.
LGTM generally, just a little nit: it seems to me that the format we use for most tests that fail in llc is ; FIXME: FILECHECK_FAIL during llvm-spirv -r in llc compilation flow. However, a few of them don't follow this style, and go for RUNx: llc -O0 -mtriple=spirv64-unknown-unknown -filetype=obj %s -o %t.llc.spv or ; FIXME: llvm-spirv -r has failed for llc compilation flow. Is there any reason for this? Should we just use the same exact thing for all of them so they're easily found through grep?
applied, also changed the description to: |
No extensions/ nor DebugInfo/ tests are touched.
For tests that are failing appropriate TODO and FIXME are added. Variants of errors:
can be found by grep LLVM_SPIRV_R_FAIL;
Note, few IR samples in llvm-spirv LITs are producing empty SPIR-V modules when compiled with SPIR-V backend due to DCE. This has to be fixed.