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

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 7 05:58:18 PST 2019


lebedev.ri added a comment.

Differential's name is slightly misleading.
Extends from i1 weren't all free before, only if they were an extensions of an `icmp` result.

Probably other arches are also affected by this (any tests failing?),
would probably be a good idea to have the same test for them too (x86, aarch64?).

And, would be awesome to precommit the adjusted tests, to see the change :)



================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:813
+      // has been removed. A target that needs it should override this
+      // method.
+      return static_cast<T *>(this)->getExtCost(cast<Instruction>(U),
----------------
'this' method? `getGEPCost()` or `getExtCost()` ?


================
Comment at: test/Analysis/CostModel/SystemZ/ext-i1-cost.ll:4-5
+;
+; Check that getUserCost() does not return TCC_Free for extensions of
+; i1.
+
----------------
This is slightly misleading.
The patch does not change the cost of an extension from i1.
It changes the cost of an extension of i1 returned from `icmp`.

So
1. Add one test where i1 was an input to the function
2. Rename test file to `ext-of-icmp-cost.ll` or something
3. Adjust comment


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

https://reviews.llvm.org/D54742





More information about the llvm-commits mailing list