[PATCH] D36462: [CGP] Fix the rematerialization of gc.relocates

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:951
   bool MadeChange = false;
+  // We must ensure the relocation of derived pointer is defined after
+  // relocation of base pointer. To achieve that we enumerate all GCRelocateInst
----------------
Doing this linear scan of the basic block once per gc-relocate seems needless expensive compile time wise.  I'd do one of the following:
1) Canonicalize the location of the base pointer before the derived relocations, run that code once per statepoint, then visit the gc-relocates only once it has run.
2) Reorganize the code to identify all remat candidates, then scan once for earliest required position, then materialize all including the moved base pointer.

Smaller comments:
- Group the BB check with the scan code since they're all handling the same case.
- Since we have to do the linear scan anyways, should we just handle the duplicate case at the same time?


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:956
+  unsigned CurBaseIdx = 0;
+  for (auto &I : *RelocatedBase->getParent())
+    if (auto RI = dyn_cast<GCRelocateInst>(&I)) {
----------------
Rather than scanning all instructions in the block, you can start at the statepoint.


https://reviews.llvm.org/D36462