[llvm-dev] Node deletion during DAG Combination ?

[Tim Northover via llvm-dev]

Hi Dominique,

On Wed, 20 Jun 2018 at 09:18, Dominique Torette via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> The idea is, during DAG combination, to merge load/extract sequence into an architecture specific node.

Interesting. My first thought would be to do that during instruction
selection rather than as a combine.

> My problem is that this node t58 is not removed from DAG. One reason could be that node t62 is still chained to t58 (due to the fact the extract_vector_elt has no chain).

Yep, you never need to actually remove nodes from the DAG yourself,
but if you're replacing one you have to make sure it's a real orphan
afterwards -- no uses accessible from the root of the DAG.

So in your case you need to make sure that not only all uses of the
extracted element are replaced, but all users of the chain attached to
the load.

> Combining: t59: f32 = extract_vector_elt t58, Constant:i16<0>
> ... into: t72: f32 = CLPISD::LOAD_VECTOR_EXTRACT_o t57:1, FrameIndex:i16<0>, Constant:i16<0>

Here it looks like you're not creating your LOAD_VECTOR_EXTRACT to
integrate into the chain in the first place. Even ignoring the
replacement issue loads need to take and produce an extra chain
argument to enforce correct ordering.

So there are 3 steps:

1. Add the incoming chain of the load you highlighted as an operand to
this new combined node.
2. Create the combined node with an output chain too (essentially just
an extra MVT::Other output value).
3. Manually call SelectionDAG::ReplaceAllUsesWith to replace all uses
of the load node's chain with this new one because the generic
combining logic only knows about the extract being combined. It can't
automatically tell you've also fiddled about with a load further up
the DAG.

That'll leave the old load with no uses in the DAG and it'll
automatically be cleaned up later.

Cheers.

Tim.