On 11/15/24 3:25 AM, Robin Dapp wrote:
So this is really the biggest question in my mind. When we kicked this
around in the patchwork meeting several weeks ago I got the impression
Robin had a correctness concern with this code. Robin, do you remember
what had you worried?
Unfortunately I haven't managed to get back to this in weeks now.
I vaguely recall that it was something related to "changed" as well but
it looks you have that covered already. I'd say let's go ahead with
that fixed.
I poked around a bit more at this.
It appears to me that at least part of the problem is we're trying to
merge vsetvl information when prev_info and curr_info are the same
object. At least that's the way it looked to me with some light
debugging. Naturally this leads to an infinite loop continually making
no real changes to the same node.
Second, my initial impression was that we should do this regardless of
whether or not the avls were compatible. But after reviewing the
codegen changes that result from such a change, I'm less sure. I didn't
see any cases where codegen got better, but I did see a few where it was
clearly worse.
I think Dusan really needs to chime in here or this can't reasonably go
forward. I'm going to mark it as deferred in patchwork until we hear
from Dusan.
jeff
