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

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 9 06:14:29 PST 2015


jfb added inline comments.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4446
@@ +4445,3 @@
+    if (!AXDead) {
+      // FIXME: If computeRegisterLiveness() reported LQR_Unknown and AX is
+      // dead even though we couldn't determine that, then we are reading an
----------------
hfinkel wrote:
> jfb wrote:
> > hfinkel wrote:
> > > This seems like an odd FIXME to keep now that we have a way to detect the situation. Why don't you detect the LQR_Unknown here and abort instead of doing something the verifier will complain about? Then we can have a FIXME that we really should be using some real analysis here instead of a local search.
> > I'm not sure I understand what you're suggesting, maybe the new FIXME isn't clear enough: you're right that a real analysis would fix the issue, but if the current one says unknown then we have to save/restore AX otherwise we may be clobbering live state. It's better to complain at verification time rather than to clobber state!
> Indeed, thanks for clarifying. You're right, we can't do better than this for now.
OK, it would be good to clarify the FIXME then, if Hal didn't get it then it's not clear enough :)


Repository:
  rL LLVM

http://reviews.llvm.org/D15320





More information about the llvm-commits mailing list