In D87224#2274955, @reames wrote:

LGTM - if you're just adding tests, you don't need separate review as long as there's nothing fancy.

In D85954#2230604, @reames wrote:

LGTM

As a follow up, it would be nice to handle two additional optimizations:

1. Canonicalize each pointer to a single index in the statepoint operand list
2. Shrink the gc-live bundle to remove any pointers not needed after (1)

ping

I've updated the patch, please take a look.

Thanks!

Please fix the bug Philip mentioned and I think we are ready to land it.

lgtm

Hi Denis, after you re-write the patch in this way, I guess we can do it clearly and simply in terms of reading.

As I understand a lot of things here are done to workout the case that one catch block can correspond to different invoke instructions. Can you please add the test examine this case?

could you please explain the case due to you need to return back in collectGCRegs implementation?

I need an answer to the question about test and at least update doc for rewriteStatepoint.

A pretty close to be done.

You are right, CFG is not modified. Thank you for the change.

Hi Alina,
to me the patch seems to be NFC.

LGTM after fixing the doc.

Doc for function should probably explicitly mention that zero value means using default option value.

Handled Fedor's comments. Thank you for review.

I'm not sure how correct this fix is but I verified that it fixes my original problem.

it is not NFC as you fix verifier. Split this patch into two parts or remove NFC.

Thanks @skatkov. The test compiles for me, and the part that this patch introduces looks correct, but there seems to be a problem with how %tmp8 is handled - as a live-out first-order-recurrence which fold-tail does not handle (the compare it introduces is not used by anyone). To reproduce the bug w/o this patch, transform the loop iv %tmp9 to start at 0 and exit the loop when equal to 4 (instead of starting at 1 and exiting at 5), and add 1 to %tmp8. Would be good to open a PR.
Continuing to investigate.

PR filed: https://bugs.llvm.org/show_bug.cgi?id=45526

Hello @Ayal, unfortunately this patch causes the functional regression.
For the test below, vectorizer decided to vectorize inner loop by 32 while it has only a couple of iteration and it causes a miscompile.
Please fix it quickly or revert the patch.

Fix.

This is the same patch as https://reviews.llvm.org/D77371, the fix will uploaded over to be able to see the difference.

Patch with a fix: https://reviews.llvm.org/D77797

In D77371#1971651, @zero9178 wrote:

This patch breaks building LLVM with BUILD_SHARED_LIBS due to a linker error. UseRegsitersForDeoptValues is declared as extern in TargetPassConfig while the definition of the value is in SelectionDAG/StatepointLower.cpp. As the LLVMCodegen target does not link against LLVMSelectionDAG it causes undefined reference errors when linking LLVMCodegen

return back option in a test

Handled comments before landing.

ok, I could not come up with something better, we can revisit at later.

From what I see it should work. The only thing worries me is that this isCorrectType seems to be not incorporated into solution but some side hack to solve the problem.
Unfortunately at this moment I do not have a better suggestion :(

Changes according to review.

in favor of https://reviews.llvm.org/D77130

Philip, thank you for your suggestion.

Please take a loot at https://reviews.llvm.org/D77122 and https://reviews.llvm.org/D77130.

In D77036#1950340, @reames wrote:

Er, I think the specification simplify needs changed here. As we've discussed offline, it's impossible to enforce having a gc pointer in the deopt list always be in the gc list (in any form). This was a flaw in the specification from the begging and we should fix the docs, not try to hack up the code to preserve it a bit longer.

The key example of why this can't work:
%p1 = bitcast i8* %p to i8*
statepoint [gc = (%p1)], [deopt = (%p1)]

The optimizer is allowed to replace either use (or both) of %p1 with %p. If it updates only one of the two (entirely legal), the two sets do not overlap.

If we need to enforce a property for GC values within deopt list, we should directly use the type of the pointer in question. Anything else appears unsound.

Need to think about other approach