Replace llvm.fmuladd.v1f* with llvm.fmuladd.f* since OpenCL doesn't support length-1 vector.

[haonanya1]

No description provided.

[CLAassistant]

All committers have signed the CLA.

[haonanya1]

@svenvh , @MrSidims, can you please take a look? Thanks.

[MrSidims]

I won't resist to merge the patch, yet my preference to fix this earlier in the compilation stack (before the translator) by replacing all <1 x ...> vectors in the module with scalars. My opinion that we better not to do such replacements in the translator. The reason is that we don't know the environment for which LLVM IR was compiled and I can imagine an unlikely yet possible situation, when LLVM IR compiled for RISC-V architecture comes as an input to the translator and AFAIK <1 x ...> vectors in LLVM IR can be used to represent scalable vectors RISC-V extension. So replacing them with scalars in the translator would be incorrect. Counterargument would be: for this RISC-V folks should add a new extension in SPIR-V, so until it's enabled we could replace vectors with scalars, but IMO it's still better not to do so and continue to emit an error.

UPD: ... LLVM IR are used ... -> ... LLVM IR can be used ...

[MrSidims]

This extension is not standard and we shouldn't rely on it. Meanwhile you have to use it as there are <1 x ...> vectors in the module.

[haonanya1]

Thanks for quick reply, @MrSidims, the IR is valid, the number of elements for vector type is a constant integer value larger than 0 in llvm ir https://llvm.org/docs/LangRef.html#vector-type. Component Count of spirv OpTypeVector must be at least 2 https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpTypeVector. There are gaps for IR and spirv. @bashbaug , what do you think? Thanks very much.

[MrSidims]

the IR is valid, the number of elements for vector type is a constant integer value larger than 0 in llvm ir

@haonanya1 that's correct. But here comes the question whether we consider https://github.com/KhronosGroup/SPIRV-LLVM-Translator as a proper backend with instructions/intrinsics lowering and types legalization or just as a translator, which just translates one IR form into another form of IR. Currently we are positioning it somewhere in between, doing lowering and not doing legalization. Yet again, it might be OK to add mapping for <1 x ...> vectors to scalars, but I'm a bit afraid to break existing or future flows as <1 x ...> can be also a scalable vector. May be it's fine to break it, at least we would know if we have such customers from incoming llvm-spirv github issue(s) :-)

I'd like to hear opinion from @svenvh . I'll propose the PR to OpenCL TSG meeting.

[svenvh]

I'm a bit afraid to break existing or future flows as <1 x ...> can be also a scalable vector

Aren't scalable vectors represented using <vscale x ...>?

I don't have strong objections against doing a vec1 -> scalar replacement in the translator. However I would suggest to implement that as a separate pass, and not only for the fmuladd intrinsic, but for all instructions in the module with <1 x ...> vector types.

[MrSidims]

Aren't scalable vectors represented using <vscale x ...>?

Oh, indeed, not sure why I though it's just <1 x ...>. So no objections from me to do such transformation.