Subzero: Improve the representation and handling of the FakeKill instruction.

src/IceCfg.cpp

@@ -316,8 +316,6 @@ bool Cfg::validateLiveness() const {
     for (Inst *Inst : Node->getInsts()) {
       if (Inst->isDeleted())
         continue;
-      if (llvm::isa<InstFakeKill>(Inst))
-        continue;
       if (FirstInst == NULL)
         FirstInst = Inst;
       InstNumberT InstNumber = Inst->getNumber();

src/IceCfgNode.cpp

@@ -667,9 +667,7 @@ void CfgNode::livenessPostprocess(LivenessMode Mode, Liveness *Liveness) {
     if (Mode == Liveness_Intervals) {
       if (InstFakeKill *Kill = llvm::dyn_cast<InstFakeKill>(I)) {
         if (!Kill->getLinked()->isDeleted()) {
-          SizeT NumSrcs = I->getSrcSize();
-          for (SizeT Src = 0; Src < NumSrcs; ++Src) {
-            Variable *Var = llvm::cast<Variable>(I->getSrc(Src));
+          for (Variable *Var : Kill->getKilledRegs()) {
             InstNumberT InstNumber = I->getNumber();
             Var->addLiveRange(InstNumber, InstNumber, 1);
           }

src/IceInst.cpp

@@ -454,11 +454,8 @@ InstFakeUse::InstFakeUse(Cfg *Func, Variable *Src)
 
 InstFakeKill::InstFakeKill(Cfg *Func, const VarList &KilledRegs,
                            const Inst *Linked)
-    : InstHighLevel(Func, Inst::FakeKill, KilledRegs.size(), NULL),
-      Linked(Linked) {
-  for (Variable *Var : KilledRegs)
-    addSource(Var);
-}
+    : InstHighLevel(Func, Inst::FakeKill, 0, NULL), KilledRegs(KilledRegs),
+      Linked(Linked) {}
 
 // ======================== Dump routines ======================== //
 
@@ -730,6 +727,8 @@ void InstUnreachable::dump(const Cfg *Func) const {
 }
 
 void InstFakeDef::emit(const Cfg *Func) const {
+  // Go ahead and "emit" these for now, since they are relatively
+  // rare.
   Ostream &Str = Func->getContext()->getStrEmit();
   Str << "\t# ";
   getDest()->emit(Func);
@@ -744,12 +743,7 @@ void InstFakeDef::dump(const Cfg *Func) const {
   dumpSources(Func);
 }
 
-void InstFakeUse::emit(const Cfg *Func) const {
-  Ostream &Str = Func->getContext()->getStrEmit();
-  Str << "\t# ";
-  Str << "use.pseudo ";
-  emitSources(Func);
-}
+void InstFakeUse::emit(const Cfg *Func) const { (void)Func; }
 
 void InstFakeUse::dump(const Cfg *Func) const {
   Ostream &Str = Func->getContext()->getStrDump();
@@ -757,21 +751,20 @@ void InstFakeUse::dump(const Cfg *Func) const {
   dumpSources(Func);
 }
 
-void InstFakeKill::emit(const Cfg *Func) const {
-  Ostream &Str = Func->getContext()->getStrEmit();
-  Str << "\t# ";
-  if (Linked->isDeleted())
-    Str << "// ";
-  Str << "kill.pseudo ";
-  emitSources(Func);
-}
+void InstFakeKill::emit(const Cfg *Func) const { (void)Func; }
 
 void InstFakeKill::dump(const Cfg *Func) const {
   Ostream &Str = Func->getContext()->getStrDump();
   if (Linked->isDeleted())
     Str << "// ";
   Str << "kill.pseudo ";
-  dumpSources(Func);
+  bool First = true;
+  for (Variable *Var : KilledRegs) {
+    if (!First)
+      Str << ", ";
+    First = false;
+    Var->dump(Func);
+  }
 }
 
 void InstTarget::dump(const Cfg *Func) const {

src/IceInst.h

@@ -801,6 +801,7 @@ public:
     return new (Func->allocateInst<InstFakeKill>())
         InstFakeKill(Func, KilledRegs, Linked);
   }
+  const VarList &getKilledRegs() const { return KilledRegs; }
   const Inst *getLinked() const { return Linked; }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg * /* Func */) const override {}
@@ -811,6 +812,7 @@ private:
   InstFakeKill(Cfg *Func, const VarList &KilledRegs, const Inst *Linked);
   ~InstFakeKill() override {}
 
+  const VarList &KilledRegs;
   // This instruction is ignored if Linked->isDeleted() is true.
   const Inst *Linked;
 };

src/IceOperand.cpp

@@ -350,18 +350,8 @@ void VariablesMetadata::addNode(CfgNode *Node) {
   for (Inst *I : Node->getInsts()) {
     if (I->isDeleted())
       continue;
-    if (InstFakeKill *Kill = llvm::dyn_cast<InstFakeKill>(I)) {
-      // A FakeKill instruction indicates certain Variables (usually
-      // physical scratch registers) are redefined, so we register
-      // them as defs.
-      for (SizeT SrcNum = 0; SrcNum < I->getSrcSize(); ++SrcNum) {
-        Variable *Var = llvm::cast<Variable>(I->getSrc(SrcNum));
-        SizeT VarNum = Var->getIndex();
-        assert(VarNum < Metadata.size());
-        Metadata[VarNum].markDef(Kind, Kill, Node);
-      }
-      continue; // no point in executing the rest
-    }
+    // Note: The implicit definitions (and uses) from InstFakeKill are
+    // deliberately ignored.
     if (Variable *Dest = I->getDest()) {
       SizeT DestNum = Dest->getIndex();
       assert(DestNum < Metadata.size());

src/IceTargetLoweringX8632.cpp

@@ -316,6 +316,8 @@ TargetX8632::TargetX8632(Cfg *Func)
 void TargetX8632::translateO2() {
   TimerMarker T(TimerStack::TT_O2, Func);
 
+  initFakeKilledScratchRegisters();
+
   if (!Ctx->getFlags().PhiEdgeSplit) {
     // Lower Phi instructions.
     Func->placePhiLoads();
@@ -411,6 +413,9 @@ void TargetX8632::translateO2() {
 
 void TargetX8632::translateOm1() {
   TimerMarker T(TimerStack::TT_Om1, Func);
+
+  initFakeKilledScratchRegisters();
+
   Func->placePhiLoads();
   if (Func->hasError())
     return;
@@ -1869,12 +1874,9 @@ void TargetX8632::lowerCall(const InstCall *Instr) {
   }
 
   // Insert a register-kill pseudo instruction.
-  VarList KilledRegs;
-  for (SizeT i = 0; i < ScratchRegs.size(); ++i) {
-    if (ScratchRegs[i])
-      KilledRegs.push_back(Func->getTarget()->getPhysicalRegister(i));
-  }
-  Context.insert(InstFakeKill::create(Func, KilledRegs, NewCall));
+  assert(!FakeKilledScratchRegisters.empty());
+  Context.insert(
+      InstFakeKill::create(Func, FakeKilledScratchRegisters, NewCall));
 
   // Generate a FakeUse to keep the call live if necessary.
   if (Instr->hasSideEffects() && ReturnReg) {
@@ -4540,8 +4542,7 @@ void TargetX8632::postLower() {
     // allocator, for example compute live ranges only for pre-colored
     // and infinite-weight variables and run the existing linear-scan
     // allocator.
-    if (llvm::isa<InstFakeKill>(Inst))
-      continue;
+    assert(!llvm::isa<InstFakeKill>(Inst) || Inst->getSrcSize() == 0);
     for (SizeT SrcNum = 0; SrcNum < Inst->getSrcSize(); ++SrcNum) {
       Operand *Src = Inst->getSrc(SrcNum);
       SizeT NumVars = Src->getNumVars();
@@ -4563,9 +4564,6 @@ void TargetX8632::postLower() {
     FreedRegisters.reset();
     if (Inst->isDeleted())
       continue;
-    // Skip FakeKill instructions like above.
-    if (llvm::isa<InstFakeKill>(Inst))
-      continue;
     // Iterate over all variables referenced in the instruction,
     // including the Dest variable (if any).  If the variable is
     // marked as infinite-weight, find it a register.  If this
@@ -4579,6 +4577,8 @@ void TargetX8632::postLower() {
     SizeT NumSrcs = Inst->getSrcSize();
     if (Dest)
       ++NumSrcs;
+    if (NumSrcs == 0)
+      continue;
     OperandList Srcs(NumSrcs);
     for (SizeT i = 0; i < Inst->getSrcSize(); ++i)
       Srcs[i] = Inst->getSrc(i);

src/IceTargetLoweringX8632.h

@@ -484,10 +484,20 @@ protected:
   SizeT NextLabelNumber;
   bool ComputedLiveRanges;
   VarList PhysicalRegisters[IceType_NUM];
+  VarList FakeKilledScratchRegisters;
   static IceString RegNames[];
 
 private:
   ~TargetX8632() override {}
+  // Ideally, this initialization would be done in the constructor,
+  // but we need to defer it until after the initial CFG is built,
+  // because some of the bitcode reader tests rely on the order that
+  // Variables are created and their default printable names.
+  void initFakeKilledScratchRegisters() {
+    for (SizeT I = 0; I < ScratchRegs.size(); ++I)
+      if (ScratchRegs[I])
+        FakeKilledScratchRegisters.push_back(getPhysicalRegister(I));
+  }
   template <typename T> void emitConstantPool() const;
 };