[PATCH] D22011: [SystemZ] Generate fewer instructions for (sub <constant>, x)

[Ulrich Weigand via llvm-commits]

uweigand added a comment.

This can lead to wrong code generation, since you now unconditionally introduce SGRK, even on platforms (like z10) that don't actually have that instruction.

Also, in general SGRK is *not* better than SGR.  Only if there is a register conflict should we use the three-address form.  Note that there is a separate pass to make that distinction.  The way this is supposed to work is that at instruction selection, we generate the two-address from, and then the two-address instruction pass switches this to the three-address form if it considers this form more profitable in any given situation.

This happens in lib/CodeGen/TwoAddressInstructionPass.cpp, in TwoAddressInstructionPass::tryInstructionTransform, which uses the subroutine TwoAddressInstructionPass::isProfitableToConv3Addr to check whether to make that transformation.  Having a quick look at that routine, it seems that it currently does not recognize the situation in your test case:

  // Look for situations like this:
  // %reg1024<def> = MOV r1
  // %reg1025<def> = MOV r0
  // %reg1026<def> = ADD %reg1024, %reg1025
  // r2            = MOV %reg1026
  // Turn ADD into a 3-address instruction to avoid a copy.
  unsigned FromRegB = getMappedReg(RegB, SrcRegMap);
  if (!FromRegB)
    return false;
  unsigned ToRegA = getMappedReg(RegA, DstRegMap);
  return (ToRegA && !regsAreCompatible(FromRegB, ToRegA, TRI));

So it converts to three-address form if, in the terms of the example above, reg1026 and reg1024 are both tied to hard registers, but different ones.  This is correct, since in that situation using a two-address form forces a copy.  But this situation does not apply in your test case.

However, there is a *different* case when a copy is forced, and that is when reg1026 and reg1025 are both tied to the same hard register, because this means that we cannot also use that same hard register for the value in reg1024, and thus we also need an extra copy for the two-address form.  This is the situation in your test case.

So I think the proper fix would be to improve that routine to detect this case as well, which should then fix the test case without requiring any SystemZ back-end changes.


http://reviews.llvm.org/D22011