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

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

In D97108#2590570 <https://reviews.llvm.org/D97108#2590570>, @reames wrote:

> In D97108#2590224 <https://reviews.llvm.org/D97108#2590224>, @skatkov wrote:
>
>> From my point of view it looks better in terms that we explicitly process deopt gc pointers as other gc values.
>> The disadvantages is that now we will have a lot of dead tied-def registers and deopt value will be listed twice in uses.
>
> This comment confused me greatly, and caused me to take a closer look at the current patch.  After doing so, I'm glad I did!  This version is unsound.
>
> For the gc pointer to remain valid until deoptimization, we must update it during any GC cycles during the lifetime of the call and before the deopt point.  To do that, we need to model the call as modifying the pointer.  Otherwise, the register allocator is free to place it in a read only location (such as an argument slot) or combine the updated and non-updated copies of value into a single register.  (Consider the case where a null check on the original unrelocated value was reordered after the safepoint.)  The fact this one doesn't cause a tied def is a symptom of a bug, not a code quality improvement.

I agree that alternative solution is better in terms of probability but here we need fixup caller saved registers anyway. 
In alternative approach we end up with something like

  tied-def %1 = statepoint (deopt %1) (gc tied-use %1)

ad in theory nothing prevents RA to allocate different registers for use in gc and use in deopt. It is unlikely but anyway.
This is just a comment.


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

https://reviews.llvm.org/D97108