[PATCH] D157624: [AMDGPU] Treat KIMM32 and KIMM16 operand types as noninlinable

Thu Aug 10 08:29:40 PDT 2023

foad added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:7823-7824
+      if (!Op.isReg() && !isInlineConstant(Op, OpInfo) &&
+          OpInfo.OperandType != AMDGPU::OPERAND_KIMM16 &&
+          OpInfo.OperandType != AMDGPU::OPERAND_KIMM32) {
         HasLiteral = true;
----------------
Joe_Nash wrote:
> arsenm wrote:
> > mbrkusanin wrote:
> > > arsenm wrote:
> > > > this part is redundant?
> > > No, because size of fmamk and fmaak (instructions that use kimm32 and kimm16 operands) is already 8 so we do not need to add extra 4 bytes for these literals.
> > Then that should be covered by !isInlineConstant.
> > 
> > The API made more sense before when it was only checking for explicitly encoded src operands. This shouldn't require special casing here, something is wrong with the operand definitions if it does
> The KIMM are not inline constants and they are not true Literals either. There needs to be a special case somewhere. I think it is better to have a special case here than spread over everywhere we have to handle literals.
Just a drive-by comment: would it help to set the size of fmaak to 4, so you *do* have to add the size of the literal to get the true size?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157624