[PATCH] D15320: CodeGen: Redo analyzePhysRegs() and computeRegisterLiveness()

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 8 10:46:34 PST 2015


MatzeB added inline comments.

================
Comment at: include/llvm/CodeGen/MachineInstrBundle.h:173
@@ +172,3 @@
+
+    /// Reg or an overlapping register is defined.
+    bool Defined;
----------------
jfb wrote:
> Can you change uses of "overlapping" here and elsewhere? I think they're part of the confusion.
Hmm, not sure what a better description would be.
What about "Reg or one of its aliases is defined. The definition may only cover parts of the register." and then
"Reg or a super-register is defined. The definition covers the full register." below?

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4454
@@ -4453,3 @@
-    //        Once fixed, also update cmpxchg-clobber-flags.ll and
-    //        peephole-na-phys-copy-folding.ll.
-
----------------
jfb wrote:
> Could you also fix peephole-na-phys-copy-folding.ll? Or does it trigger the verifier as the new FIXME says? We could pass a bigger `Neighborhood` value to `computeRegisterLiveness` if that's the case.
Yes peephole-na-phys-copy-folding is affected by the FIXME I added (I'll add the name of the test to the comment). This is still an unsolved issue before and after the changes. Increasing the Neighborhood does not help as we need an "infinite" neighborhood to cover all cases. To get a precise liveness query we would need a LivePhysRegs (or RegisterScavenger) instance which ideally is kept up to date while we walk backwards over the basic blocks unfortunately ExpandPostRAPseudos has no easy way to maintain state and walks forward at the moment, so fixing this properly is work for another patch.


Repository:
  rL LLVM

http://reviews.llvm.org/D15320





More information about the llvm-commits mailing list