Page MenuHomePhabricator

[RewriteStatepointsForGC] All constant should have null base pointer
ClosedPublic

Authored by igor-laevsky on May 24 2016, 11:38 AM.

Details

Summary

Currently we consider that each constant has itself as a base value. I.e "base(const) = const". This introduces couple of problems when we are trying to avoid reporting constants in statepoint live sets:

  1. When querying "base( phi(const1, const2) )" we will get "phi(const1, const2)" as a base pointer. Since it's not a constant we will record it in a stack map. However on practice we don't want this to happen (constant are never relocated).
  2. base( phi(const, gc ptr) ) = phi( const, base(gc ptr) ). This particular case imposes challenge on our runtime - we don't expect to see constant base pointers other than null.

This problems can be avoided by treating all constant as if they were derived from null pointer base. I.e in a first case we will not include constant pointer in a stack map at all. In a second case we will get "phi(null, base(gc ptr))" as a base pointer which is a lot more convenient.

@Manuel, I understand that your use case of the RS4GC is sensitive to how we handle global variables. Could you please take a look so that this change doesn't bring you any problems?

Diff Detail

Repository
rL LLVM

Event Timeline

igor-laevsky retitled this revision from to [RewriteStatepointsForGC] All constant should have null base pointer.
igor-laevsky updated this object.
igor-laevsky added reviewers: mjacob, sanjoy, reames.
igor-laevsky added a subscriber: llvm-commits.
mjacob edited edge metadata.May 24 2016, 1:42 PM

I don't think this will bring any problems. My collector just ignores constant bases. Note that also before this change findBaseDefiningValue() didn't necessarily actually return the right value. E.g. calling it with a GEP on a global will return the GEP instead of the global.

sanjoy accepted this revision.May 26 2016, 6:29 PM
sanjoy edited edge metadata.

lgtm

We can potentially consider doing something friendlier for cases like base-pointers-12.ll (i.e. report @global as the base pointer for those cases in the future, but for now this is fine.

This revision is now accepted and ready to land.May 26 2016, 6:29 PM
This revision was automatically updated to reflect the committed changes.