On Wednesday 30 January 2008 15:06, Evan Cheng wrote:> Hrm, I see a bug here. Let's say the liverange in question is [13,20) > and the interval it's being merged to is something like this: [1, 4), > [10, 15) > > IP = std::upper_bound(IP, end(), Start); > if (IP != begin() && IP[-1].end > Start) { > if (IP->valno != LHSValNo) { > ReplacedValNos.push_back(IP->valno); > IP->valno = LHSValNo; // Update val#. > } > > IP is end() and we would be pushing junk into ReplacedValNos. Is this > what you saw?Yep, exactly.> I wonder if the fix should be changing the inner if to: > if (IP[-1].valno != LHSValNo) { > ReplacedValNos.push_back(IP[-1].valno); > IP[-1].valno = LHSValNo; > }I was thinking along the same lines. But does this work for the cases where IP is _not_ end()? Is it true that we always want to look one back from IP? -Dave
On Jan 30, 2008, at 1:21 PM, David Greene wrote:> On Wednesday 30 January 2008 15:06, Evan Cheng wrote: >> Hrm, I see a bug here. Let's say the liverange in question is [13,20) >> and the interval it's being merged to is something like this: [1, 4), >> [10, 15) >> >> IP = std::upper_bound(IP, end(), Start); >> if (IP != begin() && IP[-1].end > Start) { >> if (IP->valno != LHSValNo) { >> ReplacedValNos.push_back(IP->valno); >> IP->valno = LHSValNo; // Update val#. >> } >> >> IP is end() and we would be pushing junk into ReplacedValNos. Is this >> what you saw? > > Yep, exactly. > >> I wonder if the fix should be changing the inner if to: >> if (IP[-1].valno != LHSValNo) { >> ReplacedValNos.push_back(IP[-1].valno); >> IP[-1].valno = LHSValNo; >> } > > I was thinking along the same lines. But does this work for the > cases where > IP is _not_ end()? Is it true that we always want to look one back > from IP?I think so. That's the intention and it's safe because IP != begin(). Evan> > > -Dave > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Wednesday 30 January 2008 16:22, Evan Cheng wrote:> > I was thinking along the same lines. But does this work for the > > cases where > > IP is _not_ end()? Is it true that we always want to look one back > > from IP? > > I think so. That's the intention and it's safe because IP != begin().Right. But doesn't that imply we've had some messed up intervals being created for quite some time, even onces where IP was _not_ end()? How the heck did this ever work? :-/ -Dave