[AMDGPU][MC] f16 inline constants are still enabled for integer operands of several opcodes

[dpreobra]

Bugzilla Link	46980
Version	trunk
OS	All
CC	@atamazov,@arsenm,@rampitec

Extended Description

A recent change 5f5f566 disabled use of fp inline constants with 16-bit integer operands. However there are several opcodes which still accept such constants:

s_sext_i32_i16
s_pack_lh_b32_b16
s_pack_ll_b32_b16
s_pack_hh_b32_b16
v_sat_pk_u8_i16  
v_sad_u16

I believe this should be corrected.

Also there are opcodes which operate with 8-bit and 4-bit data, for example, s_sext_i32_i8. Should fp inline constants be disabled for these instructions?

qsad opcodes (v_mqsad_pk_u16_u8, v_qsad_pk_u16_u8 etc) is another interesting case because these instructions accept u16x4 operands; it is not clear how fp inline constants are interpreted in this context.

Any ideas?

[dpreobra]

assigned to @dpreobra

[jwanggit86]

@dpreobra Are you saying that the compiler still generates, say, s_sext_i32_i16, with fp immediate? If so, could you pls give me an example to work with? Thanks!

[arsenm]

@dpreobra Are you saying that the compiler still generates, say, s_sext_i32_i16, with fp immediate? If so, could you pls give me an example to work with? Thanks!

It wouldn't generate it, but the assembler/dissassembler should interpret values correctly

[dpreobra]

If so, could you pls give me an example to work with?

The instruction

s_sext_i32_i16 s0, 0.5

is currently encoded as follows:

[0xf0,0x1a,0x80,0xbe]

In other words, 0.5 is encoded as a floating-point inline constant. But I believe the value 0.5 shall be encoded as a literal:

[0xff,0x1a,0x80,0xbe,0x00,0x38,0x00,0x00]

BTW, the changes introduced by @arsenm in https://github.com/llvm/llvm-project/commit/5f5f566b265db00f577ead268400d99f34ba9cdd seem to be broken by this commit. E.g. look at this file. I did not dig very deep, but the change looks suspicious.

[jwanggit86]

The recent commit mentioned above appears to be doing the opposite of what's being proposed here. For example:

 v_max3_i16 v5, v1, v2, 0.5
-// GFX10: encoding: [0x05,0x00,0x55,0xd7,0x01,0x05,0xfe,0x03,0x00,0x38,0x00,0x00]
+// GFX10: encoding: [0x05,0x00,0x55,0xd7,0x01,0x05,0xc2,0x03]

In this case, the encoding has been changed from literal to inline constant. So the question is, under what condition should a floating point constant be encoded as inline constant vs. literal. @shiltian Shilei, could you help answer this question?

[shiltian]

When I made the patch set that took all inline constant mentioned in the SPG as inline constant, I had conversation with a few folks as well as a question in Teams channel. The conclusion is, as long as it is one of those inline constant, we encode them as it is. The type doesn't matter here. In this case, 0.5 is an inline constant, so we encode it as it is. When the HW executes the instruction, it sees the binary representation of 0.5 and interpret the least significant 16 bits as i16.

[jwanggit86]

@dpreobra According to @shiltian 's comment above, 0.5 should be encoded as inline constant. In that case, I suppose this issue is no longer an issue. Your thoughts?

[jwanggit86]

@dpreobra I'm closing this issue for now. You can reopen it if you think there's a problem that needs to be solved.