spir-v reader generates GEP into vector of i1

[trenouf]

I may be on a slightly old and internally modified version of the spir-v reader, but I think nothing has changed in the area I'm looking at here.

The attached spir-v gets one element of a variable that is a vector of bool by doing an OpAccessChain and then an OpLoad of the single bool. It comes out of a recent version of glslang, and, as far as I can see, it is legal spir-v.

The spir-v reader converts that into the obvious way into a GEP into an alloca of a vector of i1, then a load from the GEPped pointer.

I can't find anything in the spec, but it appears that vector of i1 is not well supported in LLVM. See
https://bugs.llvm.org/show_bug.cgi?id=1784

Most code in LLVM assumes that a vector of i1 is packed, but the problem I see with the shader here is that at least GVN and alias analysis cannot cope with a GEP that is not to a byte boundary.

What I propose to try doing, as a local fix at least, is to make OpTypeBool i8 instead of i1, and add conversions when necessary after a cmp and before a select. I believe that standard LLVM optimizations will remove the extra code, the same as it does when a C compiler uses i32 for bool.

Note that we are not using the spirv-llvm tool chain for its full spir-v -> llvm -> spir-v functionality; we are only using the spir-v reader part of it as the front end of an LLVM-based shader compiler. Comments welcome from anyone using the tool chain in any way.
OpIsNan_TestDvec2_ng.zip

[trenouf]

I have made a further comment on the LLVM bug
https://bugs.llvm.org/show_bug.cgi?id=1784
in which I have realized that LLVM IR must only contain things that are representable on whatever target you are targeting. The frontend that generates the IR needs to be sensitive to the properties of the target.

In my case, I am targeting a real LLVM backend, in which a GEP into vector of i1 does not make any sense, as it would imply that all pointers to i1 carry a bit offset as well as a byte address.

When the spir-v reader is being used in a spir-v -> llvm -> spir-v toolchain, you could argue that it does make sense, because the spir-v being written at the backend allows it (by allowing an OpAccessChain into vector of bool). However, if the spir-v reader converts bool to i1, you would then need to fix up LLVM to allow for bit addressability.

So I still believe I need to change the spir-v reader to use i8 for bool instead of i1.

Is there an argument for banning OpAccessChain into vector of bool in the spir-v spec? We have only recently started seeing glslang generate it. Is it even possible to retrospectively "clarify" the spec like that?

[trenouf]

I have a patch for this that leaves most bools and bool vectors as i1, but uses i8 for a bool or bool vector that is in memory or in a struct or array.

[bader]

SPIR-V to LLVM IR converter tries to map original SPIR-V as close as possible. We can't assume that spir target in LLVM IR doesn't support GEP into vector of i1, but you can add this restriction if you translate to the LLVM IR with x86 target triple.
We usually handle difference between LLVM IR for spir target and real target in a separate LLVM "fixup" pass.
Did you consider this option?
In long term we probably should take into account data layout and allow users to set the target, but it's not clear to me yet if it's the only adjustment we need to make to support traslation to the targets other then spir.

[trenouf]

In addition to the problem with targets not supporting GEP into vector of i1, there is the other problem that LLVM does not support it either. The problem seems to be that things like GVN and alias analysis assume byte addressable memory. I guess they could be fixed to be bit addressable to cope with the SPIR-V reader generating GEP into vector of i1 when targeting the SPIR triple, but it's possible that such a change would be unpopular in the LLVM community as no real target needs it.

So in conclusion it is broken even when not targeting a real target.

[bader]

I know that there out-of-tree targets supporting vectors of i1, but I'm not sure how they use standard LLVM transformations.

I this case we usually customize the library and allow different types of behavior to enable multiple use cases.
Could you add an option to disable/enable promotion to ?

[AlexeySotkin]

I think it is nice to have an option to promote i1 to something suitable for a real target. But I prefer it to be disabled by default. Also should we specify width of the promoted type ?

[yxsamliu]

I agree that we should put the transformation under an option which is by default off.