[PATCH] D138133: [AMDGPU][GFX11][NFC] Refactor VOPD operands handling (part 2)

Dmitry Preobrazhensky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 16 06:53:40 PST 2022


dp added a comment.

> ! In D137952 <https://reviews.llvm.org/D137952>, @Joe_Nash wrote:
>  Now I see that it does not iterate over MCInst operands. So clearly it is confusing :). 
>  SrcIdx is an index into parsed **src **operands which is a subset of parsed operands. 
>  So maybe these renames are most accurate:
>
> getParsedSrcIndex -> getParsedOpIdxFromSrcIdx
>  SrcIdx -> CombinedSrcIdx
>  getParsedDstIndex -> getParsedOpIdxOfDst
>
> In getRegIndicies
>  I think SrcIdx is acutally ComponentSrcIdx
> Renames:
>  SrcIdx -> ComponentSrcIdx
>  getSrcIndex -> getCombinedMCSrcIdxFromComponentSrcIdx
>
> obviously this is getting really verbose, but that's probably better than confusingly named.

I have not exactly followed your rename suggestions, but I'll try to explain the rationale.

The interface provided by `ComponentLayout`, `ComponentProps` and `ComponentInfo` **always** 
takes component operand indices for input (i.e. Component::DST, Component::SRC0, etc.)
The interface provides functions to map these component indices to either MachineInstr/MCInst
operand indices or parsed operand indices. So I think the suffix "FromComponentSrcIdx" is not necessary.
Also note that there is only one MachineInstr/MCInst/parsed operands array for each component
so there is no ambiguity (no need to specify the "combined" suffix).

I renamed some functions and their operands to make the mapping clear, so now we have the following interface:

  unsigned ComponentLayout::getDstIndexInMCOperands();
  unsigned ComponentLayout::getSrcIndexInMCOperands(unsigned CompSrcIdx);
  
  unsigned ComponentLayout::getDstIndexInParsedOperands();
  unsigned ComponentLayout::getSrcIndexInParsedOperands(unsigned CompSrcIdx);
   
  unsigned ComponentProps::getCompSrcOperandsNum();
  unsigned ComponentProps::getCompParsedSrcOperandsNum();

The complete list of renames is below:

  getSrcOperandsNum        -> getCompSrcOperandsNum
  getParsedSrcOperandsNum  -> getCompParsedSrcOperandsNum
  getMandatoryLiteralIndex -> getMandatoryLiteralCompOperandIndex
  getDstIndex              -> getDstIndexInMCOperands
  getSrcIndex              -> getSrcIndexInMCOperands
  getParsedDstIndex        -> getDstIndexInParsedOperands
  getParsedSrcIndex        -> getSrcIndexInParsedOperands
  getParsedOperandIndex    -> getIndexInParsedOperands
  getInvalidOperandIndex   -> getInvalidCompOperandIndex


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

https://reviews.llvm.org/D138133



More information about the llvm-commits mailing list