[PATCH] D11370: Improved the interface of methods commuting operands, improved X86-FMA3 mem-folding&coalescing.

[Vyacheslav Klochkov via llvm-commits]

v_klochkov added a comment.

`Hi Quentin,

I appreciate the time you spend to this code-review request.

In this change-set I removed X86 FMA specific changes - that will be a separate change-set
as you asked. This totally excluded the changes from the files:

  llvm\test\CodeGen\X86\fma-commute-x86.ll
  llvm\test\CodeGen\X86\fma_patterns.ll 
  llvm\lib\Target\X86\X86InstrFMA.td 

I removed the duplicated comments for methods (header vs cpp) 
even though I personally like such duplication.
It is good to have function description in header file, but it is also 
so convenient to have it in .cpp when you're looking at the method implementation;
you can see what arguments mean, etc. without need to take a look at *.h file.

AMDGPU changes:

  I really did NOT want to do those non-obvious changes for AMDGPU.
  Avoiding the changes would let me to avoid questions and to simplify the review/approve process.
  
  Unfortunately, I had to do those changes. 
  Also, those changes cannot be separated from the interface changes I did for
  commuteInstruction() and findCommutedOpIndices().
  
  The problem I met there can be explained this way:
  1) There are some places like this:
       if (MI->IsCommutable() && TII->commuteInstruction(MI)) {}
      I.e. commuteInstruction() was called without preceding call of findCommutedOpIndices().
  2) commuteInstruction() implementation for AMDGPU can commute Reg and Imm operands.
  
  So, the solution is:
  a) To allow AMDGPU implementation of findCommutedOpIndices() to return true
      when the second operand is Imm.
      Otherwise, all calls mentioned in problem (1) above would not commute instructions.
  b) To fix SIFoldOperands.cpp/tryAddToFoldList() and add additional check there because
       Imm operand is not wanted there.
       I updated the FIXME comment there to make it more informative.

Thank you,
Slava
`


http://reviews.llvm.org/D11370