[PATCH] FIx bug in x87 FP stackifier pass

[Akira Hatanaka]

Thank you for the review, Jakob. The patch was committed in r214580.

The existing set of inline-asm tests in inline-asm-fpstack.ll looks fine to
me, but test cases for function calls might be missing. I'll see if I
should add something for them.



On Thu, Jul 31, 2014 at 9:29 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk>
wrote:

> Hi Akira,
>
> This looks good to me. It's great to delete so much code!
>
> 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.
>
> Thanks,
> /jakob
>
> On Jul 30, 2014, at 1:53 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
>
> Updated and rebased patch attached.
>
> - Trimmed the test case.
> - Added comments explaining how the stackifier distinguishes between
> register operands with "f" constraints and those with single register class
> constraints ("t" or "u").
> - Fixed a bug in FPS::handleSpecialFP where register operands with single
> class constraints were not rewritten correctly. Added a test case for the
> bug.
> - Fixed bug in X86FastISel::FastLowerCall.
>
>
> On Fri, Jul 18, 2014 at 3:04 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>
>> Ping (on behalf of Akira).
>>
>> 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.
>>
>> Can someone else more familiar with x87 stuff please review it?
>>
>> On Jul 1, 2014, at 4:55 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
>>
>> 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.
>>
>> 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:
>> INLINEASM <es:frndint> [sideeffect] [attdialect], $0:[regdef],
>> %ST0<imp-def,tied5>, $1:[reguse tiedto:$0], %ST0<tied3>, $2:[clobber],
>> %EFLAGS<earlyclobber,imp-def,dead>
>>
>> INLINEASM <es:fldcw $0> [sideeffect] [mayload] [attdialect], $0:[mem],
>> %EAX<undef>, 1, %noreg, 0, %noreg, $1:[clobber],
>> %EFLAGS<earlyclobber,imp-def,dead>
>> %FP0<def> = COPY %ST0 // %ST0 is defined by the first inline-asm
>> instruction.
>>
>> 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.
>> rdar://problem/16952634
>> <x87stackifier1.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
> <x87stackifier2.patch>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140801/9ccd04b8/attachment.html>