[PATCH] D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Yes, https://reviews.llvm.org/D36094 fixes the problem too! Here's what's going on in the test case, without any of the 2 patches:

  Legalize Machine IR for: test_stack_args_i32
  Legalizing: %vreg6<def>(p0) = G_FRAME_INDEX <fi#-1>;
  .. Already legal
  Legalizing: %vreg4<def>(s32) = G_LOAD %vreg6; mem:LD4[FixedStack-1](align=0)
  .. Already legal
  Legalizing: %vreg7<def>(p0) = G_FRAME_INDEX <fi#-2>;
  .. Already legal
  Legalizing: %vreg5<def>(s32) = G_LOAD %vreg7; mem:LD4[FixedStack-2](align=0)
  .. Already legal
  Legalizing: %vreg8<def>(s32) = G_ADD %vreg2, %vreg5;
  .. Already legal
  
  ......
  
  34378: Resume at 34378 (0 try-blocks remain)
  34379: Begin try-block
  34382: GIM_CheckFeatures(ExpectedBitsetID=5)
  34385: GIM_CheckNumOperands(MIs[0], Expected=3)
  34388: GIM_CheckOpcode(MIs[0], ExpectedOpcode=30) // Got=30
  34392: GIM_CheckType(MIs[0]->getOperand(0), TypeID=21)
  34396: GIM_CheckRegBankForClass(MIs[0]->getOperand(0), RCEnum=1)
  34400: GIM_CheckType(MIs[0]->getOperand(1), TypeID=21)
  34404: GIM_CheckRegBankForClass(MIs[0]->getOperand(1), RCEnum=1)
  34408: GIM_CheckType(MIs[0]->getOperand(2), TypeID=21)
  34412: GIM_CheckRegBankForClass(MIs[0]->getOperand(2), RCEnum=1)
  34415: GIR_BuildMI(OutMIs[0], 112)
  34419: GIR_Copy(OutMIs[0], MIs[0], 0)
  34423: GIR_Copy(OutMIs[0], MIs[0], 1)
  34427: GIR_Copy(OutMIs[0], MIs[0], 2)
  34430: GIR_AddImm(OutMIs[0], 14)
  34433: GIR_AddRegister(OutMIs[0], 0)
  34436: GIR_AddRegister(OutMIs[0], 0)
  34438: GIR_MergeMemOperands(OutMIs[0])
  34440: GIR_EraseFromParent(MIs[0])
  Converting operand: %vreg8<def>
  Converting operand: %vreg2
  Converting operand: %vreg5
  Converting operand: %noreg
  Converting operand: %noreg
  34442: GIR_ConstrainSelectedInstOperands(OutMIs[0])
  34443: GIR_DoneInto:
    %vreg8<def>(s32) = ADDrr %vreg2, %vreg5, pred:14, pred:%noreg, opt:%noreg; mem:LD4[FixedStack-2](align=0) GPR:%vreg8,%vreg2,%vreg5

With the MachineVerifier enabled for ARM, this causes it to fail on the test case.

Would it still make sense to only emit GIR_MergeMemOperands for instructions that mayLoad/mayStore? Unless I am missing something, I think not having this action when it's not required would speed up GlobalISel a tiny bit.


https://reviews.llvm.org/D36151