[PATCH] FIx bug in x87 FP stackifier pass

Jakob Stoklund Olesen stoklund at 2pi.dk
Thu Jul 31 09:29:18 PDT 2014


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/20140731/6b37b8a5/attachment.html>


More information about the llvm-commits mailing list