Subzero: Provide a macro for iterating over instruction variables.

src/IceRegAlloc.cpp

 //===- subzero/src/IceRegAlloc.cpp - Linear-scan implementation -----------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file implements the LinearScan class, which performs the linear-scan
 /// register allocation after liveness analysis has been performed.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "IceRegAlloc.h"
 
 #include "IceCfg.h"
 #include "IceCfgNode.h"
 #include "IceInst.h"
+#include "IceInstVarIter.h"
 #include "IceOperand.h"
 #include "IceTargetLowering.h"
 
 namespace Ice {
 
 namespace {
 
 // Returns true if Var has any definitions within Item's live range.
 // TODO(stichnot): Consider trimming the Definitions list similar to how the
 // live ranges are trimmed, since all the overlapsDefs() tests are whether some
@@ skipping 142 matching lines @@
         continue;
       if (const Variable *Var = Inst.getDest()) {
         if (!Var->getIgnoreLiveness() &&
             (Var->hasReg() || Var->mustHaveReg())) {
           if (LRBegin[Var->getIndex()] == Inst::NumberSentinel) {
             LRBegin[Var->getIndex()] = Inst.getNumber();
             ++NumVars;
           }
         }
       }
-      for (SizeT I = 0; I < Inst.getSrcSize(); ++I) {
-        Operand *Src = Inst.getSrc(I);
-        SizeT NumVars = Src->getNumVars();
-        for (SizeT J = 0; J < NumVars; ++J) {
-          const Variable *Var = Src->getVar(J);
-          if (Var->getIgnoreLiveness())
-            continue;
-          if (Var->hasReg() || Var->mustHaveReg())
-            LREnd[Var->getIndex()] = Inst.getNumber();
-        }
+      FOREACH_VAR_IN_INST(Var, Inst) {

[ascull, 2015/08/31 19:03:17]

Looking at how this is used I was a bit confused as I expected Var to already
exist as a variable. The Linux kernel has some macro'd for loops
(https://www.kernel.org/doc/htmldocs/kernel-api/API-list-for-each.html) and you
have to pass in the variable to use which solves the problem but leaves the
variable in the outer scope. But you still have the identifier name to used for
uniqueness.

I can't think how you can do it with:

  FOREACH_VAR_IN_INST(Variable *Var, Inst)

I get get stuck with '*Var' and can't work out how to strip the *.

[John, 2015/08/31 19:11:05]

The header file declaring the macros states that Var needs to be an undefined
id, but self documenting code would be better.
This looks redundant -- Var is always a Variable *.

Besides, how do you get rid of 'Variable *'?

[ascull, 2015/08/31 19:28:05]

It's not redundant as it shows the declaration of the variable instead of it
magically appearing. Being a C-style language comes with the expectation of
there being a clear declaration for variables.
 
As I've said both above and below, I'm not sure if this will work or how to do
it. I can get rid of 'Variable ' but not the *. I was just suggesting it as it
feels more similar to the native for each and you might have known some
preprocessor trick to get it to work.

[John, 2015/08/31 22:07:03]

It is redundant -- Types in variable declarations are always redundant when you
initialize said variable -- the compiler knows what type the variable is so
re-stating it is redundant. I did not say "wrong", "ugly", or "poor style." :)
Oh, mistyped. I wanted to know how you could get rid of the 'Variable ' (not the
*)

[ascull, 2015/08/31 22:31:56]

#define remove_variable(decl) abc##decl
#define abc_Variable /* Note: this is empty */

`remove_variable(Variable *Var)` -> `abc_Variable *Var` -> `*Var`

The limitation is that it requires you to know all the possibilities ahead of
time.

+        if (Var->getIgnoreLiveness())
+          continue;
+        if (Var->hasReg() || Var->mustHaveReg())
+          LREnd[Var->getIndex()] = Inst.getNumber();
       }
     }
   }
 
   Unhandled.reserve(NumVars);
   UnhandledPrecolored.reserve(NumVars);
   for (SizeT i = 0; i < Vars.size(); ++i) {
     Variable *Var = Vars[i];
     if (LRBegin[i] != Inst::NumberSentinel) {
       assert(LREnd[i] != Inst::NumberSentinel);
@@ skipping 88 matching lines @@
   for (auto I = Insts.begin(), E = Insts.end();
        I != E && (SpillPoint == E || FillPoint == E); ++I) {
     if (I->getNumber() == Start)
       SpillPoint = I;
     if (I->getNumber() == End)
       FillPoint = I;
     if (SpillPoint != E) {
       // Remove from RegMask any physical registers referenced during Cur's live
       // range.  Start looking after SpillPoint gets set, i.e. once Cur's live
       // range begins.
-      for (SizeT i = 0; i < I->getSrcSize(); ++i) {
-        Operand *Src = I->getSrc(i);
-        SizeT NumVars = Src->getNumVars();
-        for (SizeT j = 0; j < NumVars; ++j) {
-          const Variable *Var = Src->getVar(j);
-          if (Var->hasRegTmp())
-            Iter.RegMask[Var->getRegNumTmp()] = false;
-        }
+      FOREACH_VAR_IN_INST(Var, *I) {
+        if (Var->hasRegTmp())
+          Iter.RegMask[Var->getRegNumTmp()] = false;
       }
     }
   }
   assert(SpillPoint != Insts.end());
   assert(FillPoint != Insts.end());
   ++FillPoint;
   // TODO(stichnot): Randomize instead of find_first().
   int32_t RegNum = Iter.RegMask.find_first();
   assert(RegNum != -1);
   Iter.Cur->setRegNumTmp(RegNum);
@@ skipping 518 matching lines @@
     Str << "\n";
   }
   Str << "++++++ Inactive:\n";
   for (const Variable *Item : Inactive) {
     dumpLiveRange(Item, Func);
     Str << "\n";
   }
 }
 
 } // end of namespace Ice