search

KhronosGroup / SPIRV-LLVM-Translator

A tool and a library for bi-directional translation between SPIR-V and LLVM IR
Other
460 stars 207 forks source link

[NFC] Update complex-constexpr.ll test #2594

Closed MrSidims closed 1 month ago

MrSidims commented 1 month ago

It started to fail again after deab451e7a7f . Only bitcast to inttoptr has left in constexpr context in this test.

TODO: do code cleanup in lib/SPIRV/SPIRVLowerConstExpr.cpp

svenvh commented 1 month ago

The upstream RFC contains a list of constant expressions that will remain supported. Among those are inttoptr and ptrtoint which seem to be in this test too; so perhaps we should leave some testing in place?

asudarsa commented 1 month ago

As a first step, I think this particular test can be removed. There are other tests that do cover some of the supported const expressions. A cleanup to remove any unsupported const expressions may be warranted. Or I am also ok to react to fails if/when they crop up.

Thanks for addressing this @MrSidims

MrSidims commented 1 month ago

We still have tests like test/constexpr_phi.ll and test/constexpr_vector.ll (and may be elsewhere, which I can not find with ease using gitlog on SPIRVLowerConstExpr.cpp), both of them have bitcasts and ptrtoint used in constexpr context. I have an idea, will update PR in few minutes.

MrSidims commented 1 month ago

@svenvh @asudarsa should be slightly better. We ran out of the instructions to be used in constexpr context, but like this should be fine (bitcast from inttoptr) as it keeps original purpose of the test "calling one instruction from another to determine argument of a function call". I also simplified the test leaving "the latest" SPIR-V version there.