Created Ice::Instrumentation base class and accompanying hooks.

src/IceInstrumentation.cpp

+//===- subzero/src/IceInstrumentation.cpp - ICE intrumentation framework --===//

[Karl, 2016/06/06 22:27:03]

Spelling intrumentation?

[tlively, 2016/06/07 00:43:06]

Done.

+//
+//                        The Subzero Code Generator
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief Implements the Ice::Instrumentation class.
+///
+/// Subclasses can override particular instrumentation methods to specify how
+/// the the target program should be instrumented.
+///
+//===----------------------------------------------------------------------===//
+
+#include "IceInstrumentation.h"
+
+#include "IceCfg.h"
+#include "IceInst.h"
+#include "IceTargetLowering.h"
+
+namespace Ice {
+
+void Instrumentation::instrumentFunc(Cfg *Func) {
+  assert(Func);
+  LoweringContext Context;
+  for (CfgNode *Node : Func->getNodes()) {
+    Context.init(Node);
+    instrumentFuncStart(Context);

[Karl, 2016/06/06 22:27:02]

Since there is no implementation details, it is hard to decide if this is right.
Nor, there is no documentation (i.e. comments) in the header file to conclude
the meaning of this function.

However, you are calling it inside a basic block loop, yet the name implies it
is a start for a function. What are you trying to do?

Also, if I see start, I would expect an end.  If it is somehow setting the focus
on node None, then how about the name instrumentNodeFocus()?

Or possibly focusNode()?

[tlively, 2016/06/07 00:43:06]

This call should have been outside the loop. Fixed.

+    while (!Context.atEnd())
+      instrumentInst(Context);
+  }
+}
+
+void Instrumentation::instrumentInst(LoweringContext &Context) {
+  assert(!Context.atEnd());
+  Inst *Instr = iteratorToInst(Context.getCur());
+  switch (Instr->getKind()) {
+    case Inst::Alloca:
+      instrumentAlloca(Context, llvm::cast<InstAlloca>(Instr));
+      break;
+    case Inst::Arithmetic:
+      instrumentArithmetic(Context, llvm::cast<InstArithmetic>(Instr));
+      break;
+    case Inst::Br:
+      instrumentBr(Context, llvm::cast<InstBr>(Instr));
+      break;
+    case Inst::Call:
+      instrumentCall(Context, llvm::cast<InstCall>(Instr));
+      break;
+    case Inst::Cast:
+      instrumentCast(Context, llvm::cast<InstCast>(Instr));
+      break;
+    case Inst::ExtractElement:
+      instrumentExtractElement(Context, llvm::cast<InstExtractElement>(Instr));
+      break;
+    case Inst::Fcmp:
+      instrumentFcmp(Context, llvm::cast<InstFcmp>(Instr));
+      break;
+    case Inst::Icmp:
+      instrumentIcmp(Context, llvm::cast<InstIcmp>(Instr));
+      break;
+    case Inst::InsertElement:
+      instrumentInsertElement(Context, llvm::cast<InstInsertElement>(Instr));
+      break;
+    case Inst::IntrinsicCall:
+      instrumentIntrinsicCall(Context, llvm::cast<InstIntrinsicCall>(Instr));
+      break;
+    case Inst::Load:
+      instrumentLoad(Context, llvm::cast<InstLoad>(Instr));
+      break;
+    case Inst::Phi:
+      instrumentPhi(Context, llvm::cast<InstPhi>(Instr));
+      break;
+    case Inst::Ret:
+      instrumentRet(Context, llvm::cast<InstRet>(Instr));
+      break;
+    case Inst::Select:
+      instrumentSelect(Context, llvm::cast<InstSelect>(Instr));
+      break;
+    case Inst::Store:
+      instrumentStore(Context, llvm::cast<InstStore>(Instr));
+      break;
+    case Inst::Switch:
+      instrumentSwitch(Context, llvm::cast<InstSwitch>(Instr));
+      break;
+    case Inst::Unreachable:
+      instrumentUnreachable(Context, llvm::cast<InstUnreachable>(Instr));
+      break;
+    default:

[Roland McGrath, 2016/06/06 22:28:45]

This will silently ignore any other kinds of instructions.
If there are no other kinds, then the default: is unnecessary and leaving it out
should ensure you get a compiler warning for failing to handle a new kind if one
is added.  If there are other kinds that you explicitly do not want to do any
instrumentation for, then listing them all here with a case Inst:Foo: (all
falling through to the same break;) makes it clear that you really intended to
do nothing for those kinds.  Then you can leave out the default: to make sure
you get warnings in case you forgot any kinds.

[tlively, 2016/06/07 00:43:05]

I added an assert with documentation of why the other cases are left out. Only
high-level instructions should be instrumentable.

+      break;
+  }
+
+  Context.advanceCur();
+  Context.advanceNext();
+}
+
+} // end of namespace Ice