[PATCH] D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND

[Nirav Davé via llvm-commits]

Ah. That's reasonable. I suspect there's no actual changes to the output,
but I'll go check that. Thanks for the clarification.

On Tue, Apr 4, 2017 at 11:09 AM, Renato Golin via Phabricator <
reviews at reviews.llvm.org> wrote:

> rengolin added a comment.
>
> In https://reviews.llvm.org/D31625#718083, @niravd wrote:
>
> > This is specifically what Chandler requested on a similar previous patch
> (see https://reviews.llvm.org/D31286).
>
>
> What Chandler requested in that commit is what James is requesting now. A
> meaningful check on the generated instructions.
>
> Not nothing. Not everything. A meaningful representation of the problem,
> which normally translates to a short (3-5) sequence of instructions that
> change between bugged and non-bugged versions.
>
> Both Chandler and James are absolutely correct that checking for failure
> as well as checking for the exact sequence will generate problems later and
> it's not acceptable.
>
> If you don't know what to check for, run the program before and after your
> patch and see what changes. This will also give confidence to the reviewers
> that your patch is only changing what's required and needed, not some
> random parts of the program.
>
> cheers,
> --renato
>
>
> https://reviews.llvm.org/D31625
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170404/5e2cb79f/attachment.html>