This is an archive of the discontinued LLVM Phabricator instance.

Carry facts about nullness and undef across GC relocation
ClosedPublic

Authored by reames on Dec 10 2014, 6:36 PM.

Details

Summary

This change implements four basic optimizations:

  • If a relocated value isn't used, it doesn't need to be relocated.
  • If the value being relocated is null, relocation doesn't change that. (Technically, this might be collector specific. I don't know of one which it doesn't work for though.)
  • If the value being relocated is undef, the relocation is meaningless.
  • If the value being relocated was known nonnull, the relocated pointer also isn't null. (Since it points to the same source language object.)

I outlined other planned work in comments.

Diff Detail

Event Timeline

reames updated this revision to Diff 17136.Dec 10 2014, 6:36 PM
reames retitled this revision from to Carry facts about nullness and undef across GC relocation.
reames updated this object.
reames edited the test plan for this revision. (Show Details)
reames added a reviewer: sanjoy.
reames added a subscriber: Unknown Object (MLST).
sanjoy accepted this revision.Dec 11 2014, 12:36 PM
sanjoy edited edge metadata.

LGTM with comments addressed.

lib/Transforms/InstCombine/InstCombineCalls.cpp
1160

While I'm okay with keeping in this clause for now, I'm not comfortable with it.

What you're effectively saying here is that given any bit pattern P, there a source pattern Q such that P = relocate(Q). But there may be collectors that do not map to specific address patterns. For example, in x86-64, I think most collectors won't map any address to, say, ((void *) -1) because that is outside the 48 bit range.

1165

Nit: should this be "almost any collector"?

1167

Isn't this check the same as isa<ConstantPointerNull>(DerivedPtr)?

This revision is now accepted and ready to land.Dec 11 2014, 12:36 PM

Response to Sanjoy's comments.

lib/Transforms/InstCombine/InstCombineCalls.cpp
1160

I think this actually is legal. The bitpattern before relocation can be any bit pattern. We've got two cases:

  • it's an address on which relocation is well defined (for a given collector), in which case some unknown value (with a possibly restricted value) would be the right result. Undef is not a valid result in this case.
  • it's an address on which relocation is not well defined. i.e. we could pick it to be relocate(-1) Given this is not a well defined operation, we are free to pick any bit pattern as output. Undef is a valid result.

My argument that be that relocating undef can always be chosen to by the second case. I don't know of a collector with exactly specified relocations for any arbitrary address. Do you disagree?

1167

I *think* so, but am not entirely sure.

sanjoy added inline comments.Dec 11 2014, 4:05 PM
lib/Transforms/InstCombine/InstCombineCalls.cpp
1160

But then you're assuming that there is such a value as ((void *) -1) relocating which is undefined behavior. This is an assumption on relocation semantics which may not be true, for instance, on a 32 bit machine.

I think this property is very similar to stating gcrelocate(null) == null -- true for most practical GCs, but the "Right Thing" to do is to provide a hook in GCStrategy as you've mentioned later.

This revision was automatically updated to reflect the committed changes.