[PATCH] Statepoint infrastructure for garbage collection

Juergen Ributzka juergen at apple.com
Fri Nov 21 13:36:02 PST 2014


There are still a lot of coding standard violations. I marked a few of them ...

================
Comment at: include/llvm/CodeGen/MachineInstr.h:91
@@ -90,3 +90,3 @@
 
-  uint8_t NumMemRefs;                   // Information on memory references.
+  uint8_t NumMemRefs;                  // Information on memory references.
   mmo_iterator MemRefs;
----------------
Whitespace

================
Comment at: include/llvm/IR/Intrinsics.td:502
@@ +501,3 @@
+def int_experimental_gc_result_int : Intrinsic<[llvm_anyint_ty], [llvm_i32_ty]>;
+def int_experimental_gc_result_float : Intrinsic<[llvm_anyfloat_ty], [llvm_i32_ty]>;
+def int_experimental_gc_result_ptr : Intrinsic<[llvm_anyptr_ty], [llvm_i32_ty]>;
----------------
How does that work? Doesn't "anyfloat" the the concrete type from one of the arguments? 

================
Comment at: include/llvm/IR/Statepoint.h:1
@@ +1,2 @@
+
+#ifndef __LLVM_IR_STATEPOINT_H__
----------------
Copyright Header

================
Comment at: include/llvm/IR/Statepoint.h:2
@@ +1,3 @@
+
+#ifndef __LLVM_IR_STATEPOINT_H__
+#define __LLVM_IR_STATEPOINT_H__
----------------
I don't think we use leading and trailing "__".

================
Comment at: include/llvm/IR/Statepoint.h:25
@@ +24,3 @@
+class StatepointBase {
+  CallSiteTy callSite;
+  void *operator new(size_t, unsigned) LLVM_DELETED_FUNCTION;
----------------
CamelCase

================
Comment at: include/llvm/IR/Statepoint.h:56
@@ +55,3 @@
+    // 3 = callTarget, #callArgs, flag
+    int offset = 3;
+    assert(offset <= (int)callSite.arg_size());
----------------
CamelCase

================
Comment at: include/llvm/IR/Statepoint.h:61
@@ +60,3 @@
+  typename CallSiteTy::arg_iterator call_args_end() {
+    int offset = 3 + numCallArgs();
+    assert(offset <= (int)callSite.arg_size());
----------------
CamelCase

================
Comment at: include/llvm/IR/Statepoint.h:75
@@ +74,3 @@
+  typename CallSiteTy::arg_iterator vm_state_end() {
+    int offset = 3 + numCallArgs() + 1 + numTotalVMSArgs();
+    assert(offset <= (int)callSite.arg_size());
----------------
CamelCase

================
Comment at: include/llvm/IR/Statepoint.h:141
@@ +140,3 @@
+class GCRelocateOperands {
+  ImmutableCallSite _relocate;
+
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:134
@@ +133,3 @@
+    if (auto *FI = dyn_cast<FrameIndexSDNode>(Load->getBasePtr())) {
+      const int index = FI->getIndex();
+      auto itr = std::find(Builder.FuncInfo.StatepointStackSlots.begin(),
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:135
@@ +134,3 @@
+      const int index = FI->getIndex();
+      auto itr = std::find(Builder.FuncInfo.StatepointStackSlots.begin(),
+                           Builder.FuncInfo.StatepointStackSlots.end(), index);
----------------
ditto


================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:161
@@ +160,3 @@
+      // assignment loop.
+      SDValue loc =
+          Builder.DAG.getTargetFrameIndex(index, Incoming.getValueType());
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:192-194
@@ +191,5 @@
+/// or the actual lowering.
+static void removeDuplicatesGCPtrs(SmallVectorImpl<const Value *> &bases,
+                                   SmallVectorImpl<const Value *> &ptrs,
+                                   SmallVectorImpl<const Value *> &relocs,
+                                   SelectionDAGBuilder &Builder) {
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:198
@@ +197,3 @@
+  // This is horribly ineffecient, but I don't care right now
+  SmallSet<SDValue, 64> seen;
+
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:200
@@ +199,3 @@
+
+  SmallVector<const Value *, 64> newbases, newptrs, newrelocs;
+  for (size_t i = 0; i < ptrs.size(); i++) {
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:201-202
@@ +200,4 @@
+  SmallVector<const Value *, 64> newbases, newptrs, newrelocs;
+  for (size_t i = 0; i < ptrs.size(); i++) {
+    SDValue sd = Builder.getValue(ptrs[i]);
+    // Only add non-duplicates
----------------
ditto

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:234
@@ +233,3 @@
+
+  ImmutableStatepoint statepointOpers(&CI);
+
----------------
CamelCase

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:246-248
@@ +245,5 @@
+
+  std::vector<Value *> args;
+  CallInst::const_op_iterator arg_begin = statepointOpers.call_args_begin();
+  CallInst::const_op_iterator arg_end = statepointOpers.call_args_end();
+  args.insert(args.end(), arg_begin, arg_end);
----------------
...

================
Comment at: lib/IR/Statepoint.cpp:1
@@ +1,2 @@
+
+
----------------
Copyright Header

================
Comment at: lib/IR/Verifier.cpp:1119-1120
@@ +1118,4 @@
+      bool ok = false;
+      if (const Instruction* I = dyn_cast<Instruction>(U)) {
+        if (isStatepoint(I)) {
+          ok = true;
----------------
Would be nice to put this in a "isStatepoint(const Value *)" method instead.

http://reviews.llvm.org/D5683






More information about the llvm-commits mailing list