[PATCH] D125680: Correctly legalise stackmap operands

Tue May 17 07:59:37 PDT 2022

vext01 added a comment.

Hi Philip,

In D125680#3519150 <https://reviews.llvm.org/D125680#3519150>, @reames wrote:

> General direction looks reasonable.

Good!

> I do want to warn you that to my knowledge, no one is using stackmap or patchpoint.  I believe they've been effectively dead code for the last couple years.  statepoint is used, and should provide a superset of the stackmap/patchpoint functionality.

This makes me a little nervous. For our system to succeed we need them to work 100%!

> Code structure wise, would it be possibly to split this patch?  (Warning: I am no selectiondag expert!)  Could we add the SDNode in an NFC patch which does not legalize, and then handle legalization in a separate patch?  If we can, it might make some of the review (mapping argument orders, etc) more straight forward.

We could split the patch, but I don't see much benefit. It will mean picking the diff apart later, just to have an intermediate "still crashing" state.

I don't know what  the others think...

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

https://reviews.llvm.org/D125680