[Pack] Unneccesary mutation of the atom to clb lookup global context in packer

[AmirhosseinPoolad]

The packing code uses the free_pb function to clean up. Other than freeing some memory and internal data structures of the pb_type structure, this function also removes the lookup between the AtomBlockId and the CLB/PB. Previously this function was changed to possibly remove the Atom <-> PB from a packer-local context instead of the global context.

The issue persists for the Atom -> CLB lookup. While the packer does not change or uses this lookup, every time it calls free_pb this lookup gets modified which is completely unnecessary. However, we can't just remove this mutation from free_pb since the function is used in places other than the packer. Specifically, it is called in ClusteredNetlist::remove_block_impl.

This is the relevant part of free_pb. Notice how set_atom_clb is called to invalidate the lookup.

 /* Primitive */
        auto& atom_ctx = g_vpr_ctx.mutable_atom();
        auto blk_id = atom_pb_bimap.pb_atom(pb);
        if (blk_id) {
            //Update atom netlist mapping
            atom_ctx.mutable_lookup().set_atom_clb(blk_id, ClusterBlockId::INVALID());
            atom_pb_bimap.set_atom_pb(blk_id, nullptr);
        }
        atom_pb_bimap.set_atom_pb(AtomBlockId::INVALID(), pb);

Expected Behaviour

set_atom_clb shouldn't be called in free_pb when free_pb is used in packing code, while it should be called in other places.

Current Behaviour

set_atom_clb is used and the global context is changed in packer, which is unnecessary.

Possible Solution

You could possibly copy the part that calls set_atom_clb into ClusteredNetlist::remove_block_impl, but (ignoring the code duplication) what if someone in the future wants to use free_pb in some other place and forgets about the atom_clb lookup? It just adds a new footgun to free_pb.