[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.

Mr. Stellard,
Please review and approve the changes in 3 files owned by AMDGPU target:

  llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.h
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

All the changes (14 files, including AMDGPU) have been reviewed by Quentin Colombet.

Also, below I attached the e-mails I sent you to your e-mail at amd.com
Please see more details in it.

Thank you,
Vyacheslav Klochkov

  From: Klochkov, Vyacheslav N  
  Sent: Monday, September 14, 2015 4:46 PM
  To: '<EDITED>@amd.com'
  Cc: Klochkov, Vyacheslav N
  Subject: RE: LLVM code-review: http://reviews.llvm.org/D11370
   
  Mr. Stellard,
   
  In this e-mail I am asking you for approval for AMDGMU specific changes
  that are a small part of changes improving methods that commute operands of Machine Instructions.
   
  Please see the details below.
  Also, I will be more than happy to answer your questions if you have any.
   
  Thank you,
  Vyacheslav Klochkov
  ------------------------------------------------------------------------------------
  From: Klochkov, Vyacheslav N 
  Sent: Tuesday, September 8, 2015 4:05 PM
  To: <EDITED>@amd.com
  Cc: Klochkov, Vyacheslav N
  Subject: LLVM code-review: http://reviews.llvm.org/D11370
   
  Dear Mr. Stellard,
   
  Would you please approve the AMDGPU specific changes in this code-review tracker:
  http://reviews.llvm.org/D11370
   
  This change-set was reviewed and approved by several people:
  -        Quentin Colombet (official code-reviewer).
  -        David Kreitzer (code-review before submitting changes to Open Source community);
  -        Michael Kuperstein (code-review before submitting changes to community);
  -        Matthias Braun and Elena Demikhovsky (not official reviewers, sent some comments).
  Quentin recommended to ask for your approval for AMDGPU changes.
   
  This change-set includes changes in 14 files, 3 of 14 are AMDGPU specific:
  llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.h
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
   
   
  The change-set improves the interface of the findCommutedOpIndices() and commuteInstruction() methods.
  It did not change the code-generation for AMDGPU, all LIT tests passed.
   
  The old commuteInstruction() method did not specify the indices of the operands to be commuted.
  The main idea of the change-set is that commuteInstruction() must be able to have the commuted operands be specified explicitly.
  That is needed when a caller of commuteInstruction() method knows what operands must be commuted.
   
  For example, if 1st and 2nd ops are commutable and 1st and 3rd operands are commutable too,
  the old commuteInstruction() method did not allow to do the second commute transformation.
  The new method fixes that problem.
   
  It is still possible not to specify the operands to be commuted, but in such cases the operands to be commuted must be found by the method findCommutedOpIndices()
  (Please see the commuteInstruction() method in llvm/lib/CodeGen/TargetInstrInfo.cpp).
   
  This interface change in llvm/lib/CodeGen/TargetInstrInfo.cpp caused the need to do minor changes in target specific implementation of
  findCommutedOpIndices() and commuteInstruction().
   
  The changes in ARM, X86, PowerPC went very smooth. The changes in AMDGPU required me to do a little bit bigger changes.
   
  For AMDGPU I tuned the findCommutedOpIndices() such a way that now it returns TRUE for commutable Reg and Imm operands
  as Reg and Imm operands can be commuted in AMDGPU specific implementation of commuteInstruction() method.
  So, findCommutedOpIndices() and commuteInstruction() are in sync now, i.e. the 1st returns true when the 2nd can do the commute.
   
  The change in findCommutedOpIndices() allowed me to add an assert on one of operands in commuteInstructionImpl()  AMDGPU method.
  Also, I needed to add a simple check to SIFoldOperands.cpp to avoid Imm operands in the transformation where only Reg operands are expected.
   
  Thank you,
  Vyacheslav Klochkov


http://reviews.llvm.org/D11370