[patch] mergefunc struct result

Carlo Kok ck at remobjects.com
Wed Apr 30 00:10:01 PDT 2014


On Sun, 27 Apr 2014 23:33:30 +0200, Duncan P. N. Exon Smith  
<dexonsmith at apple.com> wrote:

> 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`.

Sort of; the ^M's were really just Windows line enters; Fixed.

>
>  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.
>

Done, new version attached.


-- 
Carlo Kok
RemObjects Software
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-mergefunc-struct-result-3.diff
Type: application/octet-stream
Size: 2896 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140430/c6eb0e85/attachment.obj>


More information about the llvm-commits mailing list