<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Akira,<div><br></div><div>This looks good to me. It's great to delete so much code!</div><div><br></div><div>I think we have pretty good test coverage for x87 code and inline asm, but you may want to take a second look at the existing tests to see of we missed any corner cases.</div><div><br></div><div>Thanks,</div><div>/jakob</div><div><br></div><div><div><div>On Jul 30, 2014, at 1:53 PM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Optima-Regular; font-size: 18px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr">Updated and rebased patch attached.<div><br></div><div>- Trimmed the test case.</div><div>- Added comments explaining how the stackifier distinguishes between register operands with "f" constraints and those with single register class constraints ("t" or "u").</div><div>- Fixed a bug in FPS::handleSpecialFP where register operands with single class constraints were not rewritten correctly. Added a test case for the bug.</div><div>- Fixed bug in X86FastISel::FastLowerCall.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 18, 2014 at 3:04 PM, Bob Wilson<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:bob.wilson@apple.com" target="_blank">bob.wilson@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;">Ping (on behalf of Akira).<div><br></div><div>I’ve looked over the patch and it seems like a very nice improvement to the way we handle x87 registers. Unfortunately I have very little experience in this area and don’t feel qualified to review it.</div><div><br></div><div>Can someone else more familiar with x87 stuff please review it?</div><div><br><div><blockquote type="cite"><div><div class="h5"><div>On Jul 1, 2014, at 4:55 PM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>> wrote:</div><br></div></div><div><div><div class="h5"><div dir="ltr"><p style="margin: 0px 0px 12px; padding: 0px; border: 0px; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-size: 13px; line-height: 18.850000381469727px;">The x87 FP stackifier pass has a bug where the liveness of an ST register defined by an inline-asm instruction is not tracked precisely.</p><p style="margin: 0px 0px 12px; padding: 0px; border: 0px; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-size: 13px; line-height: 18.850000381469727px;">This causes the assertion on line 1365 to fail when an ST register defined by an inline-asm is live across another inline-asm, as shown in the following example:</p><div style="margin: 0px; font-size: 11px; font-family: Menlo;"><span style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-size: 13px; line-height: 18.850000381469727px;">INLINEASM <es:frndint> [sideeffect] [attdialect], $0:[regdef], %ST0<imp-def,tied5>, $1:[reguse tiedto:$0], %ST0<tied3>, $2:[clobber], %EFLAGS<earlyclobber,imp-def,dead></span><br></div><p style="margin: 0px 0px 12px; padding: 0px; border: 0px; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-size: 13px; line-height: 18.850000381469727px;">INLINEASM <es:fldcw $0> [sideeffect] [mayload] [attdialect], $0:[mem], %EAX<undef>, 1, %noreg, 0, %noreg, $1:[clobber], %EFLAGS<earlyclobber,imp-def,dead><br>%FP0<def> = COPY %ST0 // %ST0 is defined by the first inline-asm instruction.</p><p style="margin: 0px 0px 12px; padding: 0px; border: 0px; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-size: 13px; line-height: 18.850000381469727px;">In order to fix this bug, the attached patch uses FP registers for function returns and inline-asm instructions where previously ST registers were being used and adds kill flags that are missing in FP register operands.</p><div style="margin: 0px; padding: 0px; border: 0px; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; font-size: 13px; line-height: 18.850000381469727px;"><a>rdar://problem/16952634</a></div></div></div></div><span><x87stackifier1.patch></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></div></blockquote></div><br></div></div><span><x87stackifier2.patch></span></div></blockquote></div><br></div></body></html>