[llvm-commits] [PATCH] Review request: Test case for indirect argument with X86FastISel

[Kaylor, Andrew]

So, I'll withdraw the patch and trust that this being in the archives is sufficient for future reference?  I'm OK with that.

-Andy

From: Chad Rosier [mailto:mcrosier at apple.com]
Sent: Thursday, August 09, 2012 7:42 PM
To: Kaylor, Andrew
Cc: Evan Cheng; Commit Messages and Patches for LLVM
Subject: Re: [llvm-commits] [PATCH] Review request: Test case for indirect argument with X86FastISel


On Aug 9, 2012, at 4:38 PM, Evan Cheng wrote:



On Aug 9, 2012, at 8:28 AM, "Kaylor, Andrew" <andrew.kaylor at intel.com<mailto:andrew.kaylor at intel.com>> wrote:


Hi Evan,

I'm not entirely clear as to the conditions that lead to the generation of the indirect argument.  I've tried reducing this test case further, and when I did the behavior disappeared.

I submitted a patch last month to fix a failure related to the indirect argument with X86FastISel (which Chad Rosier subsequently revised and committed).  At that time, Chad asked for a test case to reproduce the condition (which apparently isn't very common).  That's the motivation behind this test case.

I don't have any particular attachment to having this in the regular test suite, but it seems like having a test case available somewhere will be handy if anyone wants to handle the indirect argument on the X86FastISel rather than punting to the DAG selector, which is what currently happens.

I agree that we should have a test case for fast-isel handling of indirect argument if it's ever added. But as it is, I don't see value in adding this large test case.

I agree with Evan's point.  Regardless, thanks for your assistance Andy.



Evan



-Andy


From: Evan Cheng [mailto:evan.cheng at apple.com<http://apple.com/>]
Sent: Wednesday, August 08, 2012 8:17 PM
To: Kaylor, Andrew
Cc: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: Re: [llvm-commits] [PATCH] Review request: Test case for indirect argument with X86FastISel

The test case is still too big. Are you sure it's not possible to reduce it with bugpoint? If you can't reduce it, I don't see a lot of value for including it.

Evan

On Aug 7, 2012, at 11:31 AM, "Kaylor, Andrew" <andrew.kaylor at intel.com<mailto:andrew.kaylor at intel.com>> wrote:



The patch I sent earlier had a glitch in it.  This one is correct.

-Andy

From: llvm-commits-bounces at cs.uiuc.edu<mailto:llvm-commits-bounces at cs.uiuc.edu> [mailto:llvm-commits-bounces at cs.uiuc.edu<mailto:commits-bounces at cs.uiuc.edu>] On Behalf Of Kaylor, Andrew
Sent: Tuesday, August 07, 2012 2:05 PM
To: Evan Cheng
Cc: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: Re: [llvm-commits] [PATCH] Review request: Test case for indirect argument with X86FastISel

Hi Evan,

I don't have any particular output that I want to test for.  From my perspective, if the test doesn't crash then it passed.

The motivation for the test is that in an earlier incarnation of the X86FastISel code, indirect function arguments caused an llvm_unreachable exception.  This test exercises the code path that led there.

I have attached a new patch that I think addresses your other comments.

Thanks,
Andy

From: Evan Cheng [mailto:evan.cheng at apple.com]
Sent: Monday, August 06, 2012 1:06 PM
To: Kaylor, Andrew
Cc: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: Re: [llvm-commits] [PATCH] Review request: Test case for indirect argument with X86FastISel

The RUN line doesn't specify target arch or triple. That's going to break some buildbots. Also, please significantly reduce the test to include only the functionality it is intended to test. Right now it includes main() and other parts that are not needed. You also don't have patterns that tell us what it is you are testing.

Evan

On Aug 3, 2012, at 10:20 AM, "Kaylor, Andrew" <andrew.kaylor at intel.com<mailto:andrew.kaylor at intel.com>> wrote:

Ping.

From: llvm-commits-bounces at cs.uiuc.edu<mailto:llvm-commits-bounces at cs.uiuc.edu> [mailto:llvm-commits-bounces at cs.uiuc.edu<mailto:commits-bounces at cs.uiuc.edu>] On Behalf Of Kaylor, Andrew
Sent: Friday, July 27, 2012 1:12 PM
To: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: [llvm-commits] [PATCH] Test case for indirect argument with X86FastISel

Hi everyone,

The attached patch adds an X86 CodeGen test for the case where an indirect argument is used while FastISel was enabled.  This is the test case for a patch I previously posted here:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120709/146224.html

which was committed in r16055.

I've only seen the indirect argument being generated on Windows, but the test will pass on other platforms without the indirect argument being generated.

-Andy

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

<indirect-arg-test-2.patch>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120810/376d593e/attachment.html>