Skip to content

Commit 7ba7021

Browse files
kosarevkosarev
authored
[AMDGPU][MC] Keep MCOperands unencoded. (#158685)
We have proper encoding facilities to encode operands and instructions; there's no need to pollute the MC representation with encoding details. Supposed to be an NFCI, but happens to fix some re-encoded instruction codes in disassembler tests. The 64-bit operands are to be addressed in following patches introducing MC-level representation for lit() and lit64() modifiers, to then be respected by both the assembler and disassembler.
1 parent 5169bb4 commit 7ba7021

File tree

6 files changed

+50
-75
lines changed

6 files changed

+50
-75
lines changed

‎llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp‎

Lines changed: 12 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2436,17 +2436,8 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
24362436
case AMDGPU::OPERAND_REG_IMM_V2FP32:
24372437
case AMDGPU::OPERAND_REG_IMM_V2INT32:
24382438
case AMDGPU::OPERAND_INLINE_SPLIT_BARRIER_INT32:
2439-
if (isSafeTruncation(Val, 32) &&
2440-
AMDGPU::isInlinableLiteral32(static_cast<int32_t>(Val),
2441-
AsmParser->hasInv2PiInlineImm())) {
2442-
Inst.addOperand(MCOperand::createImm(Val));
2443-
return;
2444-
}
2445-
[[fallthrough]];
2446-
24472439
case AMDGPU::OPERAND_REG_IMM_NOINLINE_V2FP16:
2448-
2449-
Inst.addOperand(MCOperand::createImm(Lo_32(Val)));
2440+
Inst.addOperand(MCOperand::createImm(Val));
24502441
return;
24512442

24522443
case AMDGPU::OPERAND_REG_IMM_INT64:
@@ -2494,77 +2485,27 @@ void AMDGPUOperand::addLiteralImmOperand(MCInst &Inst, int64_t Val, bool ApplyMo
24942485

24952486
case AMDGPU::OPERAND_REG_IMM_INT16:
24962487
case AMDGPU::OPERAND_REG_INLINE_C_INT16:
2497-
if (isSafeTruncation(Val, 16) &&
2498-
AMDGPU::isInlinableIntLiteral(static_cast<int16_t>(Val))) {
2499-
Inst.addOperand(MCOperand::createImm(Lo_32(Val)));
2500-
return;
2501-
}
2502-
2503-
Inst.addOperand(MCOperand::createImm(Val & 0xffff));
2504-
return;
2505-
25062488
case AMDGPU::OPERAND_REG_INLINE_C_FP16:
25072489
case AMDGPU::OPERAND_REG_IMM_FP16:
2508-
if (isSafeTruncation(Val, 16) &&
2509-
AMDGPU::isInlinableLiteralFP16(static_cast<int16_t>(Val),
2510-
AsmParser->hasInv2PiInlineImm())) {
2511-
Inst.addOperand(MCOperand::createImm(Val));
2512-
return;
2513-
}
2514-
2515-
Inst.addOperand(MCOperand::createImm(Val & 0xffff));
2516-
return;
2517-
25182490
case AMDGPU::OPERAND_REG_IMM_BF16:
25192491
case AMDGPU::OPERAND_REG_INLINE_C_BF16:
2520-
if (isSafeTruncation(Val, 16) &&
2521-
AMDGPU::isInlinableLiteralBF16(static_cast<int16_t>(Val),
2522-
AsmParser->hasInv2PiInlineImm())) {
2523-
Inst.addOperand(MCOperand::createImm(Val));
2524-
return;
2525-
}
2526-
2527-
Inst.addOperand(MCOperand::createImm(Val & 0xffff));
2528-
return;
2529-
2530-
case AMDGPU::OPERAND_REG_INLINE_C_V2INT16: {
2531-
assert(isSafeTruncation(Val, 16));
2532-
assert(AMDGPU::isInlinableIntLiteral(static_cast<int16_t>(Val)));
2533-
Inst.addOperand(MCOperand::createImm(Val));
2534-
return;
2535-
}
2536-
case AMDGPU::OPERAND_REG_INLINE_C_V2FP16: {
2537-
assert(isSafeTruncation(Val, 16));
2538-
assert(AMDGPU::isInlinableLiteralFP16(static_cast<int16_t>(Val),
2539-
AsmParser->hasInv2PiInlineImm()));
2540-
2541-
Inst.addOperand(MCOperand::createImm(Val));
2542-
return;
2543-
}
2544-
2545-
case AMDGPU::OPERAND_REG_INLINE_C_V2BF16: {
2546-
assert(isSafeTruncation(Val, 16));
2547-
assert(AMDGPU::isInlinableLiteralBF16(static_cast<int16_t>(Val),
2548-
AsmParser->hasInv2PiInlineImm()));
2549-
2550-
Inst.addOperand(MCOperand::createImm(Val));
2551-
return;
2552-
}
2553-
2492+
case AMDGPU::OPERAND_REG_INLINE_C_V2INT16:
2493+
case AMDGPU::OPERAND_REG_INLINE_C_V2FP16:
2494+
case AMDGPU::OPERAND_REG_INLINE_C_V2BF16:
25542495
case AMDGPU::OPERAND_KIMM32:
2555-
Inst.addOperand(MCOperand::createImm(Literal.getLoBits(32).getZExtValue()));
2556-
return;
25572496
case AMDGPU::OPERAND_KIMM16:
2558-
Inst.addOperand(MCOperand::createImm(Literal.getLoBits(16).getZExtValue()));
2497+
Inst.addOperand(MCOperand::createImm(Val));
25592498
return;
2499+
25602500
case AMDGPU::OPERAND_KIMM64:
25612501
if ((isInt<32>(Val) || isUInt<32>(Val)) && !getModifiers().Lit64)
25622502
Val <<= 32;
25632503

25642504
Inst.addOperand(MCOperand::createImm(Val));
25652505
return;
2506+
25662507
default:
2567-
llvm_unreachable("invalid operand size");
2508+
llvm_unreachable("invalid operand type");
25682509
}
25692510
}
25702511

@@ -4830,7 +4771,7 @@ bool AMDGPUAsmParser::validateSOPLiteral(const MCInst &Inst,
48304771

48314772
unsigned NumExprs = 0;
48324773
unsigned NumLiterals = 0;
4833-
uint64_t LiteralValue;
4774+
int64_t LiteralValue;
48344775

48354776
for (int OpIdx : OpIndices) {
48364777
if (OpIdx == -1) break;
@@ -4839,7 +4780,9 @@ bool AMDGPUAsmParser::validateSOPLiteral(const MCInst &Inst,
48394780
// Exclude special imm operands (like that used by s_set_gpr_idx_on)
48404781
if (AMDGPU::isSISrcOperand(Desc, OpIdx)) {
48414782
if (MO.isImm() && !isInlineConstant(Inst, OpIdx)) {
4842-
uint64_t Value = static_cast<uint64_t>(MO.getImm());
4783+
auto OpType = static_cast<AMDGPU::OperandType>(
4784+
Desc.operands()[OpIdx].OperandType);
4785+
int64_t Value = encode32BitLiteral(MO.getImm(), OpType);
48434786
if (NumLiterals == 0 || LiteralValue != Value) {
48444787
LiteralValue = Value;
48454788
++NumLiterals;

‎llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,8 +464,9 @@ void AMDGPUMCCodeEmitter::encodeInstruction(const MCInst &MI,
464464
assert(STI.hasFeature(AMDGPU::Feature64BitLiterals));
465465
support::endian::write<uint64_t>(CB, Imm, llvm::endianness::little);
466466
} else {
467-
if (Desc.operands()[i].OperandType == AMDGPU::OPERAND_REG_IMM_FP64)
468-
Imm = Hi_32(Imm);
467+
auto OpType =
468+
static_cast<AMDGPU::OperandType>(Desc.operands()[i].OperandType);
469+
Imm = AMDGPU::encode32BitLiteral(Imm, OpType);
469470
support::endian::write<uint32_t>(CB, Imm, llvm::endianness::little);
470471
}
471472

‎llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3157,6 +3157,34 @@ bool isValid32BitLiteral(uint64_t Val, bool IsFP64) {
31573157
return isUInt<32>(Val) || isInt<32>(Val);
31583158
}
31593159

3160+
int64_t encode32BitLiteral(int64_t Imm, OperandType Type) {
3161+
switch (Type) {
3162+
default:
3163+
break;
3164+
case OPERAND_REG_IMM_BF16:
3165+
case OPERAND_REG_IMM_FP16:
3166+
case OPERAND_REG_INLINE_C_BF16:
3167+
case OPERAND_REG_INLINE_C_FP16:
3168+
return Imm & 0xffff;
3169+
case OPERAND_INLINE_SPLIT_BARRIER_INT32:
3170+
case OPERAND_REG_IMM_FP32:
3171+
case OPERAND_REG_IMM_INT32:
3172+
case OPERAND_REG_IMM_V2BF16:
3173+
case OPERAND_REG_IMM_V2FP16:
3174+
case OPERAND_REG_IMM_V2FP32:
3175+
case OPERAND_REG_IMM_V2INT16:
3176+
case OPERAND_REG_IMM_V2INT32:
3177+
case OPERAND_REG_INLINE_AC_FP32:
3178+
case OPERAND_REG_INLINE_AC_INT32:
3179+
case OPERAND_REG_INLINE_C_FP32:
3180+
case OPERAND_REG_INLINE_C_INT32:
3181+
return Lo_32(Imm);
3182+
case OPERAND_REG_IMM_FP64:
3183+
return Hi_32(Imm);
3184+
}
3185+
return Imm;
3186+
}
3187+
31603188
bool isArgPassedInSGPR(const Argument *A) {
31613189
const Function *F = A->getParent();
31623190

‎llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,6 +1718,9 @@ bool isInlinableLiteralV2F16(uint32_t Literal);
17181718
LLVM_READNONE
17191719
bool isValid32BitLiteral(uint64_t Val, bool IsFP64);
17201720

1721+
LLVM_READNONE
1722+
int64_t encode32BitLiteral(int64_t Imm, OperandType Type);
1723+
17211724
bool isArgPassedInSGPR(const Argument *Arg);
17221725

17231726
bool isArgPassedInSGPR(const CallBase *CB, unsigned ArgNo);

‎llvm/test/MC/Disassembler/AMDGPU/gfx10-vop3-literal.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
# GFX10: v_add_nc_i16 v5, v1, 0xcdab ; encoding: [0x05,0x00,0x0d,0xd7,0x01,0xff,0x01,0x00,0xab,0xcd,0xff,0xff]
5252
0x05,0x00,0x0d,0xd7,0x01,0xff,0x01,0x00,0xab,0xcd,0xff,0xff
5353

54-
# GFX10: v_ceil_f16_e64 v255, 0xabcd clamp ; encoding: [0xff,0x80,0xdc,0xd5,0xff,0x00,0x00,0x00,0xcd,0xab,0xff,0xff]
54+
# GFX10: v_ceil_f16_e64 v255, 0xabcd clamp ; encoding: [0xff,0x80,0xdc,0xd5,0xff,0x00,0x00,0x00,0xcd,0xab,0x00,0x00]
5555
0xff,0x80,0xdc,0xd5,0xff,0x00,0x00,0x00,0xcd,0xab,0xff,0xff
5656

5757
# GFX10: v_min_u16 v5, v1, 0xabcd ; encoding: [0x05,0x00,0x0b,0xd7,0x01,0xff,0x01,0x00,0xcd,0xab,0xff,0xff]

‎llvm/test/MC/Disassembler/AMDGPU/gfx8-literal16.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,17 @@
3434
0xff 0x06 0x02 0x3e 0x00 0x01 0x00 0x00
3535

3636
# non-zero unused bits in constant
37-
# VI: v_add_f16_e32 v1, 0x41, v3 ; encoding: [0xff,0x06,0x02,0x3e,0x41,0x00,0x01,0x00]
37+
# VI: v_add_f16_e32 v1, 0x41, v3 ; encoding: [0xff,0x06,0x02,0x3e,0x41,0x00,0x00,0x00]
3838
0xff 0x06 0x02 0x3e 0x41 0x00 0x01 0x00
3939

40-
# VI: v_add_f16_e32 v1, 0x41, v3 ; encoding: [0xff,0x06,0x02,0x3e,0x41,0x00,0x00,0x01]
40+
# VI: v_add_f16_e32 v1, 0x41, v3 ; encoding: [0xff,0x06,0x02,0x3e,0x41,0x00,0x00,0x00]
4141
0xff 0x06 0x02 0x3e 0x41 0x00 0x00 0x01
4242

4343
# FIXME: This should be able to round trip with literal after instruction
4444
# VI: v_add_f16_e32 v1, 0, v3 ; encoding: [0x80,0x06,0x02,0x3e]
4545
0xff 0x06 0x02 0x3e 0x00 0x00 0x00 0x00
4646

47-
# VI: v_add_f16_e32 v1, 0xffcd, v3 ; encoding: [0xff,0x06,0x02,0x3e,0xcd,0xff,0xff,0xff]
47+
# VI: v_add_f16_e32 v1, 0xffcd, v3 ; encoding: [0xff,0x06,0x02,0x3e,0xcd,0xff,0x00,0x00]
4848
0xff 0x06 0x02 0x3e 0xcd 0xff 0xff 0xff
4949

5050
# VI: v_mul_lo_u16_e32 v2, 0xffcd, v2 ; encoding: [0xff,0x04,0x04,0x52,0xcd,0xff,0xff,0xff]

0 commit comments

Comments
 (0)