Skip to content

Commit

Permalink
BIR: Be more judicious about flow order when deleting iblocks
Browse files Browse the repository at this point in the history
This has caused a variety of problems in the past (and I'm probably
missing stuff that will cause problems in the future). In the
course of optimization we will sometimes try to delete blocks that
are already dead code, i.e. not in the flow order. This is
unavoidable or at least nontrivial to avoid. When we do it, we
need to not modify the flow order. Modifying the flow order
improperly can lead to e.g. a cyclic flow order, which makes passes
hang. Very bad. I don't have a stable example, unfortunately, but
observed this while adding optimizations to Clasp.
  • Loading branch information
Bike committed Nov 14, 2023
1 parent 18ffb79 commit 502289f
Showing 1 changed file with 10 additions and 13 deletions.
23 changes: 10 additions & 13 deletions BIR/graph-modifications.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -399,21 +399,18 @@ See MAYBE-DELETE-IBLOCK"
(next (%next iblock)))
;; Clasp bug #1260 demonstrates a situation in which we accidentally
;; called this function on an iblock which was not in fact in the flow
;; order to begin with. These asserts check that it is.
;; ...but unfortunately, we can't use them, because they're triggered
;; by some correct code duplications as well, such as when delete-iblock
;; recurses into a loop. I have tried a few simple deletion checks but
;; they failed for various reasons (e.g. variable deletion can implicate
;; iblocks that have already been deleted, somehow). FIXME.
#+(or)
(assert (eq (%next prev) iblock))
(when prev
;; order to begin with. This can happen in correct code duplications,
;; such as when delete-iblock recurses into a loop.
;; So in order to avoid incorrect flow orders we only change the order
;; when the block is actually in the order.
(when (and prev (eq (%next prev) iblock))
(setf (%next prev) next))
(cond (next
#+(or)
(assert (eq (%prev next) iblock))
(setf (%prev next) prev))
(t (setf (tail (function iblock)) prev)))))
(when (eq (%prev next) iblock)
(setf (%prev next) prev)))
(t
(when (eq (tail (function iblock)) iblock)
(setf (tail (function iblock)) prev))))))

;;; Insert the new iblock into the flow order after AFTER.
(defun insert-iblock-into-flow-order (new after)
Expand Down

0 comments on commit 502289f

Please sign in to comment.