[PATCH] [GC] CodeGenPrep transform: simplify offsetable relocate

Philip Reames listmail at philipreames.com
Wed Jan 14 15:14:52 PST 2015


LGTM. I assume you want me to commit on your behalf?


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:616
@@ +615,3 @@
+
+    // Create a Builder and replace the target callsite with a gep
+    IRBuilder<> Builder(ToReplace);
----------------
What's here is functional, and LGTM for submission.  

As a *separate* change, you could clarify the insertion logic slightly if you wanted to.  I'd suggest either:
BasicBlock::iterator next(BaseRelocate);
while( isGCRelocate(*next)) next++;
IRBuilder<> Builder(&*next);

OR

find the last relocate in the statepoint sequence once, then pass that in as the insert point for all relocates you visit.  

Either makes it lot more clear what's going on.

================
Comment at: test/Transforms/CodeGenPrepare/statepoint-relocate.ll:23
@@ +22,3 @@
+; CHECK: getelementptr i32* %base, i32 12
+; CHECK: getelementptr i32* %base-new, i32 15
+; CHECK: getelementptr i32* %base-new, i32 12
----------------
You might want to add a CHECK: gc.statepoint in between to make the grouping of GEPs more clear.  Entirely optional.

http://reviews.llvm.org/D6883

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list