[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