[PATCH] D23253: [X86] Generalized transformation of `definstr gr8; movzx gr32, gr8` to `xor gr32, gr32; definstr gr8`

[bryant via llvm-commits]

bryant added a comment.

In https://reviews.llvm.org/D23253#522534, @qcolombet wrote:

> Hi,


Thanks for actually reviewing this.

> What is the performance impact (both runtime and compile time) of the pass?


Run-time impact, according to IACA, would be 1% to 40% (`pre-ra-sched.ll`) faster. Never worse. I'll measure the compile-time perf tonight. Do you have any suggestions on the preferred way to do this?

> You list results for test/CodeGen/X86, which is not usually what we use for perform testing. Could you try on the LLVM test suite?


My measurements have thus far centered around the performance of generated code. Since this pass only ever runs when -march=x86/-64, there would be no difference in the rest of the arches, improvements or otherwise. If you really want, I could run the rest of the suite but the result would be a bunch of passes and xfails. What sort of results do you really want me to record here?

> Moreover, like Michael said, we should try to merge this pass with something else. A possible approach would be a peephole like optimization where we can plug more patterns.


As I've mentioned before, this depends on virtual register LiveInterval information that is lost after the rewrite pass.

> Cheers,

> -Quentin





================
Comment at: lib/CodeGen/CalcSpillWeights.cpp:46-48
@@ -45,5 +45,5 @@
 // Return the preferred allocation register for reg, given a COPY instruction.
-static unsigned copyHint(const MachineInstr *mi, unsigned reg,
-                         const TargetRegisterInfo &tri,
-                         const MachineRegisterInfo &mri) {
+unsigned VirtRegAuxInfo::copyHint(const MachineInstr *mi, unsigned reg,
+                                  const TargetRegisterInfo &tri,
+                                  const MachineRegisterInfo &mri) {
   unsigned sub, hreg, hsub;
----------------
qcolombet wrote:
> This seems wrong to export this interface.
> External users should rely on MachineRegisterInfo::getSimpleHint.
> 
> What is the problem in using MachineRegisterInfo::getSimpleHint?
In cases where there is more than one hint, getSimpleHint only returns one and misses the rest.


Repository:
  rL LLVM

https://reviews.llvm.org/D23253