Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com>, Nathan =?utf-8?q?Gauër?= <brio...@google.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/107...@github.com>
VyacheslavLevytskyy wrote: I didn't dive deeply into the subject, but I'd be eager to assist in a somewhat indirect manner. I've tried the PR locally to check this with SYCL and machine verifier (expensive checks: on). The former luckily didn't notice any changes, but the latter gives some insights. Probably I see one potential issue to address with respect to the validity of code between passes, namely after SPIRV pre legalizer. 37 of 63 test cases in the 'structurizer' folder are failing when running with 'llc -verify-machineinstrs', and it looks like the repeat offender is "Virtual register defs don't dominate all uses", like in ``` ... %45:iid(s32) = ASSIGN_TYPE %59:iid(s32), %56:type(s64) %95:iid(s32) = nsw G_ADD %96:iid, %97:iid %96:iid(s32) = GET_ID %45:iid(s32) %97:iid(s32) = GET_ID %26:iid(s32) %47:iid(s32) = nsw ASSIGN_TYPE %95:iid(s32), %56:type(s64) G_STORE %47:iid(s32), %5:pid(p0) :: (store (s32) into %ir.i) G_BR %bb.9 ... *** Bad machine code: Virtual register defs don't dominate all uses. *** - function: main - v. register: %97 ``` No existing tests with '-verify-machineinstrs' start to fail, so I guess there are no problems with code modifications wrt. the general/existing branching logic, but rather with the new use case introduced by the PR/added instructions. Another weird thing is that llvm specifically build with expensive checks on is now hangs (or maybe hits a very long timeout, I haven't yet a patience to check) while running our test suite, so I'm not quite sure if this PR introduce new issues for Machine Verifier in general, in the part of existing test cases where we haven't yet inserted '-verify-machineinstrs'. https://github.com/llvm/llvm-project/pull/107408 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
