[PATCH] D52316: [TargetRegisterInfo] Remove temporary hook enableMultipleCopyHints()

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 167616.
jonpa added a comment.

I think the recent discoveries of compile time regressions were due to multiple copy hints being added for the same register many times.

The CopyHint is inserted into a set, and I must have assumed that the hint weight would most likely be the same for the same physical register, and therefore there should only be one CopyHint per reg. This was not the case in the test case provided by Jonathan where a lot of hints were added, since this happens even with just a tiny difference in the float spill weight. This test case seemed to have thousands of small intervals for some phys regs, so in this case that meant a lot of hints for that register.

I updated the patch to make sure that the sorted set of CopyHints only has one hint per register. This is achieved by checking at insertion if the register already had a hint, and if so, remove that hint and insert the new one, if the new hint has a greater weight.

This luckily seems to be completely NFC, so all tests are still passing.

In fact, the temporary hack (which this patch removes) really should be removed since it is now clear that its ordering of the hints with the temporary CopyHintOrder++ method (for targets that do not enable), is also getting a lot of duplicated hints for the same register.

Possibly, I could first handle this issue with a patch without removing the target hook if there is any reason to do so. It seems however this is "general goodness" as Quentin pointed out earlier, so there shouldn't be any target desiring to not enable, right (in-trunk or not)?


https://reviews.llvm.org/D52316

Files:
  include/llvm/CodeGen/TargetRegisterInfo.h
  lib/CodeGen/CalcSpillWeights.cpp
  lib/Target/AArch64/AArch64RegisterInfo.h
  lib/Target/AMDGPU/AMDGPURegisterInfo.h
  lib/Target/ARM/ARMBaseRegisterInfo.h
  lib/Target/BPF/BPFRegisterInfo.h
  lib/Target/Hexagon/HexagonRegisterInfo.h
  lib/Target/Mips/MipsRegisterInfo.h
  lib/Target/PowerPC/PPCRegisterInfo.h
  lib/Target/Sparc/SparcRegisterInfo.h
  lib/Target/SystemZ/SystemZRegisterInfo.h
  lib/Target/X86/X86RegisterInfo.h
  lib/Target/XCore/XCoreRegisterInfo.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52316.167616.patch
Type: text/x-patch
Size: 10334 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180929/a0e8b077/attachment.bin>