[amdgpu] some VOP3 instructions not defined as commutable

[yxsamliu]

some VOP3 instructions are commutable but are not defined as such. e.g.

# RUN: llc -mtriple=amdgcn -mcpu=gfx1030 -run-pass=machine-cse -verify-machineinstrs %s -o - 2>&1 | FileCheck --check-prefix=GCN %s
---
name: test_machine_cse_op_sel
body: |
  bb.0:
    %0:vgpr_32 = IMPLICIT_DEF
    %1:vgpr_32 = IMPLICIT_DEF
    %2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0, implicit $mode, implicit $exec
    %3:vgpr_32 = V_ADD_NC_U16_e64 0, %1, 0, %0, 0, 0, implicit $mode, implicit $exec
    DS_WRITE2_B32_gfx9 undef %4:vgpr_32, %2, %3, 0, 1, 0, implicit $exec
...

V_ADD_NC_U16_e64 is commutable but are not merged by machine-cse.

This issue was found during reviewing #106920

Two changes are needed:

1. td files need to be updated for commutable VOP3 insts
2. SIInstrInfo::commuteInstructionImpl need to be fixed to handle op_sel

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: Yaxun (Sam) Liu (yxsamliu)

some VOP3 instructions are commutable but are not defined as such. e.g.

# RUN: llc -mtriple=amdgcn -mcpu=gfx1030 -run-pass=machine-cse -verify-machineinstrs %s -o - 2>&1 | FileCheck --check-prefix=GCN %s
---
name: test_machine_cse_op_sel
body: |
  bb.0:
    %0:vgpr_32 = IMPLICIT_DEF
    %1:vgpr_32 = IMPLICIT_DEF
    %2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0, implicit $mode, implicit $exec
    %3:vgpr_32 = V_ADD_NC_U16_e64 0, %1, 0, %0, 0, 0, implicit $mode, implicit $exec
    DS_WRITE2_B32_gfx9 undef %4:vgpr_32, %2, %3, 0, 1, 0, implicit $exec
...

V_ADD_NC_U16_e64 is commutable but are not merged by machine-cse.

This issue was found during reviewing #106920

Two changes are needed:

1. td files need to be updated for commutable VOP3 insts
2. SIInstrInfo::commuteInstructionImpl need to be fixed to handle op_sel

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Yaxun (Sam) Liu (yxsamliu)

some VOP3 instructions are commutable but are not defined as such. e.g.

# RUN: llc -mtriple=amdgcn -mcpu=gfx1030 -run-pass=machine-cse -verify-machineinstrs %s -o - 2>&1 | FileCheck --check-prefix=GCN %s
---
name: test_machine_cse_op_sel
body: |
  bb.0:
    %0:vgpr_32 = IMPLICIT_DEF
    %1:vgpr_32 = IMPLICIT_DEF
    %2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0, implicit $mode, implicit $exec
    %3:vgpr_32 = V_ADD_NC_U16_e64 0, %1, 0, %0, 0, 0, implicit $mode, implicit $exec
    DS_WRITE2_B32_gfx9 undef %4:vgpr_32, %2, %3, 0, 1, 0, implicit $exec
...

V_ADD_NC_U16_e64 is commutable but are not merged by machine-cse.

This issue was found during reviewing #106920

Two changes are needed:

1. td files need to be updated for commutable VOP3 insts
2. SIInstrInfo::commuteInstructionImpl need to be fixed to handle op_sel

[easyonaadit]

Hey @yxsamliu,
I'm just starting out in Compilers, and would love to try my hand at this issue.
Could you please assign it to me?

[yxsamliu]

Hey @yxsamliu, I'm just starting out in Compilers, and would love to try my hand at this issue. Could you please assign it to me?

Thanks for looking into this.

[easyonaadit]

Hey, I had a doubt.
%2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0
in this V_ADD_NC_U16_e64 instruction, what are the 4 numbers given as modifiers supposed to stand for? I have tried looking at documentation but can't seem to find any reference to them.
%2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0
what I'm talking about is the 0's which are in bold above. Could you please provide some links to where I can read more about them?

[vg0204]

Hey, I had a doubt. %2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0 in this V_ADD_NC_U16_e64 instruction, what are the 4 numbers given as modifiers supposed to stand for? I have tried looking at documentation but can't seem to find any reference to them. %2:vgpr_32 = V_ADD_NC_U16_e64 0, %0, 0, %1, 0, 0 what I'm talking about is the 0's which are in bold above. Could you please provide some links to where I can read more about them?

Refer this link, it might help you!

[arsenm]

I usually look at the fully expanded tablegen definitions to figure out the operand structure. If you extract an llvm-tblgen invocation out of the build log, and remove the -gen-* argument, you can look at the fully expanded tablegen and find the relevant instruction. In this case, that is:

def V_ADD_NC_U16_e64 {  // InstructionEncoding Instruction AMDGPUInst PredicateControl InstSI VOP SIMCInstr VOP_Pseudo VOP3_Pseudo VOP3InstBase
 field bit isRegisterLoad = 0;
  field bit isRegisterStore = 0;
  field bits<96> SoftFail = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
  field bit SALU = 0; 
  field bit VALU = 1;
  field bit SOP1 = 0;
  field bit SOP2 = 0;
  field bit SOPC = 0;
  field bit SOPK = 0;
  field bit SOPP = 0;
  field bit VOP1 = 0;
  field bit VOP2 = 0;
  field bit VOPC = 0;
  field bit VOP3 = 1;
  field bit VOP3P = 0;
  field bit VINTRP = 0;
  field bit SDWA = 0;
  field bit DPP = 0;
  field bit TRANS = 0;
  field bit MUBUF = 0;
  field bit MTBUF = 0; 
  field bit SMRD = 0;
  field bit MIMG = 0;
  field bit VIMAGE = 0;
  field bit VSAMPLE = 0;
  field bit EXP = 0;
  field bit FLAT = 0;
  field bit DS = 0;
  field bit Spill = 0;
  field bit LDSDIR = 0;
  field bit VINTERP = 0;
  field bit VM_CNT = 0;
  field bit EXP_CNT = 0;
  field bit LGKM_CNT = 0;
  field bit WQM = 0;
  field bit DisableWQM = 0;
  field bit Gather4 = 0;
  field bit ScalarStore = 0; 
  field bit FixedSize = 0;
  field bit VOP3_OPSEL = 1;
  field bit maybeAtomic = 1;
  field bit FPClamp = 0;
  field bit IntClamp = 1;
  field bit ClampLo = 1;
  field bit ClampHi = 0;
  field bit IsPacked = 0;
  field bit D16Buf = 0;
  field bit FlatGlobal = 0;
  field bit ReadsModeReg = 0;
  field bit FPDPRounding = 0;
  field bit FPAtomic = 0;
  field bit IsMAI = 0;
  field bit IsDOT = 0;
  field bit FlatScratch = 0;
  field bit IsAtomicNoRet = 0;
  field bit IsAtomicRet = 0;
  field bit IsWMMA = 0;
  field bit TiedSourceNotRead = 0;
  field bit IsNeverUniform = 0;
  field bit GWS = 0;
  field bit IsSWMMAC = 0;
  int Size = 8;
  string DecoderNamespace = "AMDGPU";
  list<Predicate> Predicates = [isGFX10Plus];
  string DecoderMethod = "";
  bit hasCompleteDecoder = 1;
  string Namespace = "AMDGPU";
  dag OutOperandList = (outs anonymous_15962:$vdst);
  dag InOperandList = (ins IntOpSelMods:$src0_modifiers, VSrc_b16:$src0, IntOpSelMods:$src1_modifiers, VSrc_b16:$src1, Clamp0:$clamp, op_sel0:$op_sel);
  string AsmString = "";
  EncodingByHwMode EncodingInfos = ?;
  list<dag> Pattern = [(set i16:$vdst, (add (i16 (VOP3OpSelMods i16:$src0, i32:$src0_modifiers)), (i16 (VOP3OpSelMods i16:$src1, i32:$src1_modifiers))))];
  list<Register> Uses = [EXEC];
  list<Register> Defs = [];
  int CodeSize = 0;
  int AddedComplexity = -1000;
  bit isPreISelOpcode = 0;
  bit isReturn = 0;
  bit isBranch = 0;
  bit isEHScopeReturn = 0;
  bit isIndirectBranch = 0;
  bit isCompare = 0;
  bit isMoveImm = 0;
  bit isMoveReg = 0;
  bit isBitcast = 0;
  bit isSelect = 0;
  bit isBarrier = 0;
  bit isCall = 0;
  bit isAdd = 0;
  bit isTrap = 0;
  bit canFoldAsLoad = 0;
  bit mayLoad = 0;
  bit mayStore = 0;
  bit mayRaiseFPException = 0;
  bit isConvertibleToThreeAddress = 0;
  bit isCommutable = 0;
  bit isTerminator = 0;
  bit isReMaterializable = 0;
  bit isPredicable = 0;
  bit isUnpredicable = 0;
  bit hasDelaySlot = 0;
  bit usesCustomInserter = 0;
  bit hasPostISelHook = 1;
  bit hasCtrlDep = 0;
  bit isNotDuplicable = 0;
  bit isConvergent = 0;
  bit isAuthenticated = 0;
  bit isAsCheapAsAMove = 0;
  bit hasExtraSrcRegAllocReq = 1;
  bit hasExtraDefRegAllocReq = 0;
  bit isRegSequence = 0;
  bit isPseudo = 1;
  bit isMeta = 0;
  bit isExtractSubreg = 0;
  bit isInsertSubreg = 0;
  bit variadicOpsAreDefs = 0;
  bit hasSideEffects = 0;
  bit isCodeGenOnly = 1;
  bit isAsmParserOnly = 0;
  bit hasNoSchedulingInfo = 0;
  InstrItinClass Itinerary = NullALU;
  list<SchedReadWrite> SchedRW = [Write32Bit];
  string Constraints = "";
  string DisableEncoding = "";
  string PostEncoderMethod = "";
  bits<64> TSFlags = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0 };
  string AsmMatchConverter = "cvtVOP3OpSel";
  string TwoOperandAliasConstraint = "";
  string AsmVariantName = "VOP3";
  bit UseNamedOperandTable = 1;
  bit UseLogicalOperandMappings = 0;
  bit FastISelShouldIgnore = 0;
  bit HasPositionOrder = 0;
  Predicate SubtargetPredicate = isGFX10Plus;
  Predicate AssemblerPredicate = TruePredicate;  Predicate WaveSizePredicate = TruePredicate;
  True16PredicateClass True16Predicate = NoTrue16Predicate;
  list<Predicate> OtherPredicates = [];
  string OpName = "v_add_nc_u16";
  string PseudoInstr = "v_add_nc_u16_e64";
  int Subtarget = -1;
  string Mnemonic = "v_add_nc_u16";
  Instruction Opcode = V_ADD_NC_U16_e64;
  bit IsTrue16 = 0;
  VOPProfile Pfl = anonymous_24949;
  string AsmOperands = "$vdst, $src0, $src1$op_sel$clamp";
  bit HasFP8DstByteSel = 0;
}

The relevant piece is:
dag OutOperandList = (outs anonymous_15962:$vdst);
dag InOperandList = (ins IntOpSelMods:$src0_modifiers, VSrc_b16:$src0, IntOpSelMods:$src1_modifiers, VSrc_b16:$src1, Clamp0:$clamp, op_sel0:$op_sel);

So the immediate 0s are src0_modifiers, src1_modifiers, clamp, and op_sel

[easyonaadit]

Hey Matt,
Thanks so much for this, I had no idea this was possible!
I was a bit caught up in work for the past few days, I will start tinkering with this now.