[PATCH] D129634: [X86] Fix updating LiveVariables in convertToThreeAddress

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:1390-1391
     NewMI = MIB;
-
+    if (LV && SrcReg != Src.getReg())
+      LV->getVarInfo(SrcReg).Kills.push_back(NewMI);
     break;
----------------
MatzeB wrote:
> foad wrote:
> > I'm not proud of this patch because it adds duplicated code like this after almost every call to classifyLEAReg. There may be better ways to do this, by completely rethinking the way that convertToThreeAddress updates LiveVariables.
> > 
> > On the other hand LiveVariables is supposed to go away in favour of LiveIntervals, so does it really matter?
> Can't we just use the already computed `isKill` here?
Yes this works. I didn't do it that way because it will add duplicates in the Kills lists, because the call to replaceKillInstruction below will also add them (in the case that SrcReg == Src.getReg(), i.e. classifyLEAReg did not create a new register), but that does not seem to cause any problems in practice. What do you prefer?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129634/new/

https://reviews.llvm.org/D129634