On Jan 30, 2009, at 1:52 PM, David Greene wrote:> On Friday 30 January 2009 15:10, David Greene wrote: > >> This still looks correct. The coalescer then says: >> >> 4360 %reg1177<def> = FsMOVAPSrr %reg1176<kill> ; srcLine 0 >> Inspecting %reg1176,0 = [2702,4362:0) 0 at 2702-(4362) and >> %reg1177,0 >> [2700,3712:0)[3768,3878:0)[4362,4372:0) 0 at 4362-(3878): >> Joined. Result = %reg1177,0 = [2700,4372:0) 0 at 2702-(4362) >> >> Eh? How can it coalesce these two? Doesn't %reg1176:[2702,4362:0) >> overlap >> %reg1177:[2700,3712:0)? >> >> I will look into this further. Just wanted to check in and make >> sure. > > Ok, the Coalescer thinks %reg1177 value number 0 is defined by a > copy from > %reg1176. I guess that could be considered correct because %reg1177 > is > defined by a copy from %reg1176 in bb108. It thus considers the > intervals to > not interfere. > > But this can't be right. I think the problem is that there should > be two > value numbers for %reg1177. We already have VN 0 defined from > %reg1176. > What coalescing is missing is that %reg1177 is ALSO defined by an > implicit def > from bb134. That's the value number we're missing and is why > coalescing > incorrectly removed the copy.I don't have the whole context to understand why you think this is a bug. An implicit_def doesn't actually define any value. So we don't care if a live interval overlaps live ranges defined by an implicit_def. That said, the way we models undef in machine instruction has always bugged me. I thought about adding a MachineOperand type to represent undef. Then we don't have to muddle the semantics of a "def". To me, that's a cleaner representation, but it will require work. Evan> > > So it appears we can't get rid of the IMPLICIT DEF. Evan, do you > agree? > > -Dave > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Friday 30 January 2009 16:54, Evan Cheng wrote:> I don't have the whole context to understand why you think this is a > bug. An implicit_def doesn't actually define any value. So we don't > care if a live interval overlaps live ranges defined by an implicit_def.It's a bug because the coalerscer does illegal coaescing. Our last episode left us here: bb134: 2696 %reg1645<def> = FsMOVAPSrr %reg1458<kill> ; srcLine 0 bb74: 2700 %reg1176<def> = FsMOVAPSrr %reg1645<kill> ; srcLine 0 [deleted copy] 2708 %reg1178<def> = FsMOVAPSrr %reg1647<kill> ; srcLine 0 *** u before d 2712 TEST64rr %reg1173, %reg1173, %EFLAGS<imp-def> ; srcLine 30 2716 JLE mbb<file test.f90, bb90,0x3c37ed0>, %EFLAGS<imp-use,kill> ; srcLine 0 bb108: [...] 4352 %reg1253<def> = MAXSSrr %reg1253, %reg1588<kill> ; srcLine 60 4356 %reg1645<def> = FsMOVAPSrr %reg1253<kill> ; srcLine 0 4360 %reg1177<def> = FsMOVAPSrr %reg1176<kill> ; srcLine 0 *** updated 4364 %reg1647<def> = FsMOVAPSrr %reg1243<kill> ; srcLine 0 4368 JMP mbb<file test.f90, bb74,0x3c63a60> ; srcLine 0 This still looks correct. The coalescer then says: 4360 %reg1177<def> = FsMOVAPSrr %reg1176<kill> ; srcLine 0 Inspecting %reg1176,0 = [2702,4362:0) 0 at 2702-(4362) and %reg1177,0 = [2700,3712:0)[3768,3878:0)[4362,4372:0) 0 at 4362-(3878): Joined. Result = %reg1177,0 = [2700,4372:0) 0 at 2702-(4362) Now let's look at the resulting code: bb134: 2696 %reg1645<def> = FsMOVAPSrr %reg1458<kill> ; srcLine 0 bb74: 2700 %reg1177<def> = FsMOVAPSrr %reg1645<kill> ; srcLine 0 *** u [deleted copy] 2708 %reg1178<def> = FsMOVAPSrr %reg1647<kill> ; srcLine 0 *** u before d 2712 TEST64rr %reg1173, %reg1173, %EFLAGS<imp-def> ; srcLine 30 2716 JLE mbb<file test.f90, bb90,0x3c37ed0>, %EFLAGS<imp-use,kill> ; srcLine 0 bb108: [...] 4352 %reg1253<def> = MAXSSrr %reg1253, %reg1588<kill> ; srcLine 60 4356 %reg1645<def> = FsMOVAPSrr %reg1253<kill> ; srcLine 0 [deleted copy] 4364 %reg1647<def> = FsMOVAPSrr %reg1243<kill> ; srcLine 0 4368 JMP mbb<file test.f90, bb74,0x3c63a60> ; srcLine 0 The very first instruction in bb74 is wrong. The coalescer has said that y always has the same value of x and that's incorrect. y is always one value "behind" x in the original source. The coalescer thinks it can do this because %reg1177 only has one value number and that VN is marked as a copy from %reg1176. What I'm saying is that while %reg1177 really is copied from %reg1176, it also has some initial value ("undef") coming into the loop. That value is not captured by the live interval information. It's because of this value that we cannot coalesce %reg1177 and %reg1176. It's because of this value that %reg1177 is always one value "behind" %reg1176. Now, if there's some other way to tell the coalescer that the coalescing is illegal, that's fine. I don't care about the undef value number itself. I care about the coalescer behaving itself. :) I just don't know how to tell the coalescer not to coalesce this without disabling all coalescings of interfering live intervals, even those that are just copies from the source register. Can you think of another way to fix this that's quick and easy?> That said, the way we models undef in machine instruction has always > bugged me. I thought about adding a MachineOperand type to represent > undef. Then we don't have to muddle the semantics of a "def". To me, > that's a cleaner representation, but it will require work.Long-term I don't have an opinion on what happens here. But right now I need to fix this. -Dave
On Feb 2, 2009, at 10:08 AM, David Greene wrote:> On Friday 30 January 2009 16:54, Evan Cheng wrote: > >> I don't have the whole context to understand why you think this is a >> bug. An implicit_def doesn't actually define any value. So we don't >> care if a live interval overlaps live ranges defined by an >> implicit_def. > > It's a bug because the coalerscer does illegal coaescing. > > Our last episode left us here: > > bb134: > 2696 %reg1645<def> = FsMOVAPSrr %reg1458<kill> ; srcLine 0 > > bb74: > 2700 %reg1176<def> = FsMOVAPSrr %reg1645<kill> ; srcLine 0 > [deleted copy] > 2708 %reg1178<def> = FsMOVAPSrr %reg1647<kill> ; srcLine > 0 *** u > before d > 2712 TEST64rr %reg1173, %reg1173, %EFLAGS<imp-def> ; srcLine 30 > 2716 JLE mbb<file test.f90, bb90,0x3c37ed0>, %EFLAGS<imp- > use,kill> ; > srcLine > 0 > > bb108: > [...] > 4352 %reg1253<def> = MAXSSrr %reg1253, %reg1588<kill> ; > srcLine 60 > 4356 %reg1645<def> = FsMOVAPSrr %reg1253<kill> ; srcLine 0 > 4360 %reg1177<def> = FsMOVAPSrr %reg1176<kill> ; srcLine > 0 *** > updated > 4364 %reg1647<def> = FsMOVAPSrr %reg1243<kill> ; srcLine 0 > 4368 JMP mbb<file test.f90, bb74,0x3c63a60> ; srcLine 0 > > This still looks correct. The coalescer then says: > > 4360 %reg1177<def> = FsMOVAPSrr %reg1176<kill> ; srcLine 0 > Inspecting %reg1176,0 = [2702,4362:0) 0 at 2702-(4362) > and > %reg1177,0 > [2700,3712:0)[3768,3878:0)[4362,4372:0) 0 at 4362-(3878): > Joined. Result = %reg1177,0 = [2700,4372:0) 0 at 2702- > (4362) > > Now let's look at the resulting code: > > bb134: > 2696 %reg1645<def> = FsMOVAPSrr %reg1458<kill> ; srcLine 0 > > bb74: > 2700 %reg1177<def> = FsMOVAPSrr %reg1645<kill> ; srcLine 0 > *** u > [deleted copy] > 2708 %reg1178<def> = FsMOVAPSrr %reg1647<kill> ; srcLine > 0 *** u > before d > 2712 TEST64rr %reg1173, %reg1173, %EFLAGS<imp-def> ; srcLine 30 > 2716 JLE mbb<file test.f90, bb90,0x3c37ed0>, %EFLAGS<imp- > use,kill> ; > srcLine > 0 > > bb108: > [...] > 4352 %reg1253<def> = MAXSSrr %reg1253, %reg1588<kill> ; > srcLine 60 > 4356 %reg1645<def> = FsMOVAPSrr %reg1253<kill> ; srcLine 0 > [deleted copy] > 4364 %reg1647<def> = FsMOVAPSrr %reg1243<kill> ; srcLine 0 > 4368 JMP mbb<file test.f90, bb74,0x3c63a60> ; srcLine 0 > > The very first instruction in bb74 is wrong. The coalescer has said > that y > always has the same value of x and that's incorrect. y is always > one value > "behind" x in the original source. > > The coalescer thinks it can do this because %reg1177 only has one > value number > and that VN is marked as a copy from %reg1176. What I'm saying is > that while > %reg1177 really is copied from %reg1176, it also has some initial > value > ("undef") coming into the loop. That value is not captured by the > live > interval information. It's because of this value that we cannot > coalesce > %reg1177 and %reg1176. It's because of this value that %reg1177 is > always one > value "behind" %reg1176.I am sorry I don't really follow it. Is this what you are describing? %v1177 = undef ... loop: ... %v1176 = op ... = %v1177 %v1177 = %v1176 jmp loop Why is not safe to coalesce the 2 registers? Evan> > > Now, if there's some other way to tell the coalescer that the > coalescing is > illegal, that's fine. I don't care about the undef value number > itself. I > care about the coalescer behaving itself. :) I just don't know how > to tell > the coalescer not to coalesce this without disabling all coalescings > of > interfering live intervals, even those that are just copies from the > source > register. > > Can you think of another way to fix this that's quick and easy? > >> That said, the way we models undef in machine instruction has always >> bugged me. I thought about adding a MachineOperand type to represent >> undef. Then we don't have to muddle the semantics of a "def". To me, >> that's a cleaner representation, but it will require work. > > Long-term I don't have an opinion on what happens here. But right > now I > need to fix this. > > -Dave