[patch] mergefunc struct result

[Duncan P. N. Exon Smith]

On 2014 Apr 22, at 00:45, Carlo Kok <ck at remobjects.com> wrote:

> On Sun, 20 Apr 2014 01:50:49 +0200, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>>> 
>>> In this case no, the function works on ints and pointers now:
>> 
>> For clarity, I mean:
>> 
>>    if (SrcTy->isStructTy()) {
>>      assert(DestTy->isStructTy() && "Can only cast SrcTy to StructTy");
>>      // ...
>>    }
>>    assert(!DestTy->isStructTy() && "Cannot cast SrcTy to StructTy");
>> 
> 
> 
> Ah that makes more sense. Attached is a version with all your suggestions integrated & clang formatted. The only issue I had was that from within Visual Studio the testcases refused to test at all:
> 
> 20>  WindowsError: [Error 193] %1 is not a valid Win32 application (TaskId:167)
> 
> I did run the opt/filecheck commands manually before and after and it properly failed (before) and succeeded after.

I applied your patch and ran the test locally, and it seems to work.

I think you should add `assert(!DestTy->isStructTy())` after the `if`
statement.

Otherwise, this is looking good.  Just a few oddities in the test to
clean up.

 1. Remove the semi-colon in `mergefunc-struct-return.ll` after your
    declaration of `@noop`.  In IR, semi-colons start comments, but
    you don't have a comment there.

 2. In your testcase, I'm seeing `^M` at the end of every line.
    Please remove the linefeeds.  The testcase is missing a newline
    at the end of the file, too, but I'm guessing it has a linefeed.
    Make sure it has a newline (but don't add an empty line...).
    There's also a trailing space character in `@fn1` (after the
    closing `}`).  All of these issues would be detected by
    `git diff --check`.

 3. Add a `CHECK-LABEL: @fn1(`.

 4. Add a comment at the top of the file (after the `RUN` line) to
    describe what the test is about.