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

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 4 08:09:35 PDT 2017


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





More information about the llvm-commits mailing list