[PATCH] D125680: Correctly legalise stackmap operands

Edd Barrett via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 18 07:17:45 PDT 2022


vext01 added a comment.

> Please do not touch statepoint in this patch. If you do not really need it, leave it alone at all.

Just to clarify why I've proposed this:

`llvm.experimental.stackmap`, `llvm.experimental.patchpoint` and`llvm.experimental.gc.statepoint` used to all share the same DAG building code via a function called `addStackMapLiveVars()` in `SelectionDAGBuilder.cpp`.

This function incorrectly emits target nodes, thus erroneously side-stepping the legalisation stage.

My change (so-far) detaches only `llvm.experimental.stackmap` from this function.

Ideally, all three facilities would use a common function that emits target-independent nodes when legalisation is required, but if upstream is against that, we should at the very least add a big flashing neon message to the effect of:

  // XXX: this is incorrect because it emits target nodes, meaning that operands do not get legalised.

Are we on the same page?


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

https://reviews.llvm.org/D125680



More information about the llvm-commits mailing list