[PATCH] D84098: [AMDGPU][MC] Corrected decoding of 16-bit literals

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp marked 3 inline comments as done.
dp added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/add.i16.ll:39
 ; VI: flat_load_ushort [[A:v[0-9]+]]
-; VI: v_add_u16_e32 [[ADD:v[0-9]+]], 0xfffffcb3, [[A]]
+; VI: v_add_u16_e32 [[ADD:v[0-9]+]], -0x34d, [[A]]
 ; VI-NEXT: buffer_store_short [[ADD]]
----------------
arsenm wrote:
> dp wrote:
> > arsenm wrote:
> > > We currently don't match source modifiers on integer instructions, so there's no situation where a - should appear here
> > This is not a modifier. '-' is a part of constant. It is handled uniformly for integer and fp operands. See a comment at AMDGPUAsmParser::parseSP3NegModifier:
> > 
> > // Currently this modifier is allowed in the following context:
> > //
> > // 1. Before a register, e.g. "-v0", "-v[...]" or "-[v0,v1]".
> > // 2. Before an 'abs' modifier: -abs(...)
> > // 3. Before an SP3 'abs' modifier: -|...|
> > //
> > // In all other cases "-" is handled as a part
> > // of an expression that follows the sign.
> > 
> > Do you prefer 0xFFFFFFFFFFFFFCB3?
> Yes, but truncated to the 32-bit value, so 0xffffcb3
Why do you want it to be truncated to 32 bits? Note that all assembler constants are 64 bit.

Currently we have uniform truncation rules which work for 16-bit and 32-bit literals. They have been working fine for several years. See https://llvm.org/docs/AMDGPUOperandSyntax.html#conversion-of-integer-values

Do you propose to have separate truncation rules for 16 bit values? This does not seem logical and changing the rules may break existing code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84098/new/

https://reviews.llvm.org/D84098