Make pnacl-bccompress add abbreviations for obvious constants.

lib/Bitcode/NaCl/Reader/NaClBitCodes.cpp

Index: lib/Bitcode/NaCl/Reader/NaClBitCodes.cpp
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitCodes.cpp b/lib/Bitcode/NaCl/Reader/NaClBitCodes.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..54f2c1ba08a5708df6fc4fff9cdd5dca68f044bd
--- /dev/null
+++ b/lib/Bitcode/NaCl/Reader/NaClBitCodes.cpp
@@ -0,0 +1,97 @@
+//===- NaClBitcodeHeader.cpp ----------------------------------------------===//
+//     PNaCl bitcode header reader.
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Implementation of Bitcode abbrevations.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Bitcode/NaCl/NaClBitCodes.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+
+void NaClBitCodeAbbrevOp::Print(raw_ostream& Stream) const {
+  if (isLiteral()) {
+    Stream << getLiteralValue();
+  } else if (isEncoding()) {
+    switch (getEncoding()) {
+    case Fixed:
+      Stream << "Fixed(" << getEncodingData() << ")";
+      break;
+    case VBR:
+      Stream << "VBR(" << getEncodingData() << ")";
+      break;
+    case Array:
+      Stream << "Array";
+      break;
+    case Char6:
+      Stream << "Char6";
+      break;
+    case Blob:
+      Stream << "Blob";
+      break;
+    default:
+      Stream << "??";

[jvoung (off chromium), 2014/02/06 20:37:19]

This shouldn't be possible right?

[Karl, 2014/02/07 19:37:29]

Yes. Adding assert to cause failure.

+      break;
+    }
+  } else {
+    Stream << "??";

[jvoung (off chromium), 2014/02/06 20:37:19]

Should be impossible?

[Karl, 2014/02/07 19:37:29]

Yes. Adding assert to cause failure.

+  }
+}
+
+static void PrintExpression(raw_ostream &Stream,
+                            const NaClBitCodeAbbrev *Abbrev,
+                            unsigned &Index) {
+  // Bail out early, in case we are incrementally building the
+  // expression and the argument is not available yet.
+  if (Index >= Abbrev->getNumOperandInfos()) return;
+
+  const NaClBitCodeAbbrevOp &Op = Abbrev->getOperandInfo(Index);
+  Op.Print(Stream);
+  if (unsigned NumArgs = Op.NumArguments()) {
+    Stream << "(";
+    for (unsigned i = 0; i < NumArgs; ++i) {
+      ++Index;
+      if (i > 0) Stream << ",";
+      PrintExpression(Stream, Abbrev, Index);
+    }
+    Stream << ")";
+  }
+}
+
+void NaClBitCodeAbbrev::Print(raw_ostream &Stream) const {
+  Stream << "[";
+  for (unsigned i = 0; i < getNumOperandInfos(); ++i) {
+    if (i > 0) Stream << ", ";
+    PrintExpression(Stream, this, i);
+  }
+  Stream << "]\n";
+}
+
+NaClBitCodeAbbrev *NaClBitCodeAbbrev::Simplify() const {
+  NaClBitCodeAbbrev *Abbrev = new NaClBitCodeAbbrev();
+  for (unsigned i = 0; i < OperandList.size(); ++i) {
+    const NaClBitCodeAbbrevOp &Op = OperandList[i];
+    // Simplify if possible.  Currently, the only simplification known
+    // is to remove unnecessary operands appearing immediately before an
+    // array operator. That is, apply the simplification:
+    //    Op Array(Op) -> Array(Op)
+    while (Abbrev->OperandList.size() >= 2
+           && Abbrev->OperandList.back().isArrayOp()

[jvoung (off chromium), 2014/02/06 20:37:19]

How can the NaClBitCodeAbbrevOp::Array be the last thing in the list of
operands? Doesn't some other encoding op need to show up after, to know the type
of the array elements? You seem to be looking for the type of the array element
before the NaClBitCodeAbbrevOp::Array instead. Also, there isn't a restriction
on where the first "Op" in the sequence "Op Array(Op)" comes, since you are
starting from index 0?

Or am I understanding this wrong?

Does this come up because of something bccompress does?
It doesn't happen in the default set of abbrevs right?

[Karl, 2014/02/07 19:37:29]

This is a nested loop, where we are incrementally copying elements from this to
Abbrev. As such, the array element can appear last (i.e. added by the last
iteration). Apparently this wasn't clear. Rewriting to make this more explicit
(and simplier).

[jvoung (off chromium), 2014/02/07 21:34:10]

Ah, I mixed up the "this" Abbrev and the new Abbrev...

+           && Abbrev->OperandList[Abbrev->OperandList.size()-2] == Op) {
+      Abbrev->OperandList.pop_back();
+      Abbrev->OperandList.pop_back();
+      Abbrev->OperandList.push_back(
+          NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
+    }
+    Abbrev->OperandList.push_back(Op);
+  }
+  return Abbrev;
+}