[PATCH] D97108: [Statepoint Lowering] Allow dead gc pointer from deopt section to be on register.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Serguei,

>From your comment, it sounds like we're already assuming values in the deopt list are base pointers fairly widely.  (e.g. even the current stack lowering makes this assumption)   Your explanation matches my vague memory, so I don't have any reason to question that.

With that in mind, I think it's okay to continue with that assumption, but I'd like to make that assumption as explicit as we can manage.  (And document it explicitly!)

Glancing at the code, I want to suggest an alternate approach.  I won't require this, but I'm curious to hear what you think.

At the MARKed location, what if we inserted something along the lines of the following:

  // If we find a deopt value which isn't explicitly added, we need to
  // ensure it gets lowered such that gc cycles occurring before the
  // deoptimization event during the lifetime of the call don't invalidate
  // the pointer we're deopting with.  Note that we assume that all
  // pointers passed to deopt are base pointers; relaxing that assumption
  // would require relatively large changes to how we represent relocations.
  for (Value *V : I.deopt_operands()) {
    if (!isGCValue(V)) continue;
    if (Seen.insert(V).second) {
      SI.Bases.push_back(V);
      SI.Ptrs.push_back(V);
    }
  }

I think this has the same effect, but is much more explicit about what is going on.

Your take?



================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:1053
     }
   }
 
----------------
MARK (see overall comment)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97108/new/

https://reviews.llvm.org/D97108