[PATCH] D54742: [CodeMetrics] Don't let extends of i1 be free.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

In D54742#1316748 <https://reviews.llvm.org/D54742#1316748>, @RKSimon wrote:

> @chandlerc Are you happy with this now?


Not really. The test is still too indirect and complex IMO.

As I said in my comments, there are *direct* tests of the cost model, including the `getUserCost` result. Not sure why that got ignored, I gave the path for them. A concrete example:

  llvm/test/Analysis/CostModel/X86/costmodel.ll

I don't think this should be tested by complex inlining tests. Instead, we have a direct and obvious way to test this stuff and we should be using it. The fact that other architectures are *only* testing the latency or reciprocal throughput cost model, and not the code size cost model, is not a reason to bypass the clear and obvious testing layer we have here. Instead we should add exactly the test for the changed cost model that this change is proposing.


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

https://reviews.llvm.org/D54742