Support llvm.frexp intrinsic translation

[vmaksimo]

Map @llvm.frexp intrinsic to OpenCL Extended Instruction frexp builtin.

The difference in signatures and return values is covered by extracting/combining values from and into composite type.

LLVM IR:
{ float %fract, i32 %exp }  @llvm.frexp.f32.i32(float %val)
SPIR-V:
{ float %fract } ExtInst frexp (float %val, i32 %exp)

[vmaksimo]

Please take a look @MrSidims @asudarsa @LU-JOHN @bwlodarcz @VyacheslavLevytskyy

[MrSidims]

This change also fixes reverse translation of non-constant values of OpCompositeConstruct.

There is a value to put it in a separate PR. Reason is that the bug fix alone is a good candidate to be backported to earlier release branches

[vmaksimo]

There is a value to put it in a separate PR. Reason is that the bug fix alone is a good candidate to be backported to earlier release branches

I'll try to come up with a PRs split.

[asudarsa]

Please take a look @LU-JOHN @bwlodarcz @VyacheslavLevytskyy

Thanks

[bwlodarcz]

During review I checked what will happen when:

define { float, i1 } @frexp_bool() {
  %ret = call { float, i1 } @llvm.frexp.f32.i1(float -0.0)
  ret { float, i1 } %ret
}

which is correct llvm IR.
It appears that the result SPIRV seems ok:

7 Decorate 6 LinkageAttributes "frexp_bool" Export
...

4 TypeInt 15 32 0
3 TypeFloat 3 32
2 TypeBool 4
4 TypeStruct 2 3 4

3 TypeFunction 5 2
4 TypePointer 8 7 4
4 TypeStruct 14 3 15
....

5 Function 2 6 0 5

2 Label 7
4 Variable 8 9 7
7 ExtInst 3 11 1 frexp 10 9
4 Load 4 12 9
5 CompositeConstruct 2 13 11 12
2 ReturnValue 13

1 FunctionEnd

but looking on OpenCL extension spec:
https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html

halfn frexp (halfn x, __global intn *exp)
half frexp (half x, __global int *exp)

halfn frexp (halfn x, __local intn *exp)
half frexp (half x, __local int *exp)

halfn frexp (halfn x, __private intn *exp)
half frexp (half x, __private int *exp)

For OpenCL C 2.0 or with the __opencl_c_generic_address_space feature macro:

halfn frexp (halfn x, intn *exp)
half frexp (half x, int *exp)

Is this correct? I mean can we pass BoolType pointer to OpenCL frexp and expect to be handled correctly?

[MrSidims]

Is this correct? I mean can we pass BoolType pointer to OpenCL frexp and expect to be handled correctly?

No, we mustn't expect that. And I do believe, that it is LLVM bug - it makes a little sense for frexp to store integral exponent of a float to i1 (unless you want to support mini-floats, but it's not part of LLVM and it's unlikely to be inherited from just float).

I suggest that we add here an assertion (or an error, but IMHO assertion here is better) to check if input integer is not i1. For the rest integer types we should add conversion to i32 by UConvert to obey the spec https://registry.khronos.org/SPIR-V/specs/1.0/OpenCL.ExtendedInstructionSet.100.mobile.html "exp must be a pointer(global, local, private, generic) to i32 or vector(2,3,4,8,16) of i32 values".

[bwlodarcz]

Is this correct? I mean can we pass BoolType pointer to OpenCL frexp and expect to be handled correctly?

No, we mustn't expect that. And I do believe, that it is LLVM bug - it makes a little sense for frexp to store integral exponent of a float to i1 (unless you want to support mini-floats, but it's not part of LLVM and it's unlikely to be inherited from just float).

I suggest that we add here an assertion (or an error, but IMHO assertion here is better) to check if input integer is not i1. For the rest integer types we should add conversion to i32 by UConvert to obey the spec https://registry.khronos.org/SPIR-V/specs/1.0/OpenCL.ExtendedInstructionSet.100.mobile.html "exp must be a pointer(global, local, private, generic) to i32 or vector(2,3,4,8,16) of i32 values".

Why just not use everywhere i32 and just trunc or zext exponent result?

[asudarsa]

Is this check in conformance with comments? Do we want to assert out if type is i16?
Also two other comments:

1. Can we pls add a test for this?
2. Can we add a error message here?

Thanks

[vmaksimo]

Do we want to assert out if type is i16?

Yes, because the intrinsic is used only with i32 by OpenCL Ext Inst set specification. And actually LLVM language reference also allows only i32 here.

1. Can we pls add a test for this?

OK, I can add a separate negative test for that.

2. Can we add a error message here?

I agree with Dmitry that the assertion is better in this case. Do you mind if I leave it as is?

[asudarsa]

Looks good. few comments to address.

Thanks

[LU-JOHN]

Do we want to make this CHECK-SPIRV-DAG in case the order changes in the future?

[vmaksimo]

Not sure about that - I don't think that types would be shuffled.
I guess we should proceed as is and add DAG checks only if the ordering problem appears.

[LU-JOHN]

LGTM

[MrSidims]

Restarting CI, merging afterwards