[llvm-commits] [PATCH] Fix PR11004: Crash in llc arm backend

[David Meyer]

Cameron,

This one small change (AlwaysInline: false -> true) can keep tip-of-tree
functional while Dan works on nested call legalization. It will be easy to
reverse this change when his fix goes in.

IMHO, it is inconsiderate to break the tree intentionally, especially when a
temporary fix is so easy. Even if you XFAIL tests, there will be people who
run into the regression in other ways, and may be forced to spend time
debugging it.

In addition to my bug, there are two other bugs filed by others which may be
related:

http://llvm.org/bugs/show_bug.cgi?id=10876
http://llvm.org/bugs/show_bug.cgi?id=10518

I've attached a new patch, with a TODO and without a new test.

- pdox




On Sun, Sep 25, 2011 at 3:26 PM, Cameron Zwarich <zwarich at apple.com> wrote:

> Dan removed support (temporarily, hopefully) for nested call legalization
> in r138977 and then XFAIL'd the test
> test/CodeGen/Generic/2010-11-04-BigByval.ll on ARM in r139058. The latter
> test hits the same infinite recursion as your test case when compiled for
> ARM.
>
> Cameron
>
> On Sep 25, 2011, at 10:46 AM, David Meyer wrote:
>
> +Gohman
>
> Cameron,
>
> A nested callseq is currently an invalid DAG representation, and results in
> a crash or an infinite loop later in compilation. I don't see how it could
> ever result in a valid program.
>
> If somebody intends to improve ARM performance by making a nested call to
> memcpy possible, that is great. But in the meantime, these memcpys really
> need to be inlined so that the compiler works at all.
>
> - David
>
>
> On Sun, Sep 25, 2011 at 3:46 AM, Cameron Zwarich <zwarich at apple.com>wrote:
>
>> The fact that the memcpy is not always inlined is an explicit design
>> decision of the ARM byval implementation. Think of the case of copying a
>> gigantic struct.
>>
>> The nested legalization of calls it required is unfortunate and has bugs,
>> but I believe Dan Gohman has been cleaning it up a bit.
>>
>> Unless I am just really out of the loop (which is possible) this is not
>> the right fix.
>>
>> Sent from my iPhone
>>
>> On Sep 25, 2011, at 1:34 AM, David Meyer <pdox at google.com> wrote:
>>
>> > Fixes http://llvm.org/bugs/show_bug.cgi?id=11004
>> >
>> > New test included. Seeking approval to commit.
>> >
>> > Thanks,
>> > pdox
>> > <arm_byval.patch>
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110925/dd40ea9d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm_byval_2.patch
Type: application/octet-stream
Size: 1038 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110925/dd40ea9d/attachment.obj>