[PATCH] D54824: [PowerPC] [NFC] Add test cases to the ISD::BR_CC node in the instruction selection

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

In D54824#1306039 <https://reviews.llvm.org/D54824#1306039>, @steven.zhang wrote:

> Personally, I don't like writing the test case using such script. Because, it will check all the instructions and make your test point unclear. And it hard code the register name, which make this test fragile。


This makes maintenance of tests easier since anyone can re-run the script. Of course, the test case is very strictly specified which will make it sensitive to changes in codegen/regalloc/scheduling. However, it is arguably easier for someone to re-run the script and for reviewers to carefully review every aspect of the change than it would be if only specific instructions are checked. There is some value in tests being sensitive to changes not necessarily related to what they are testing - we may discover unintended changes that are actually detrimental.

As to the point about what the test case is actually testing, I think that is a valid concern and a comment should explain that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54824/new/

https://reviews.llvm.org/D54824