[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 17 13:38:40 PDT 2020


nikic added a comment.

In D87188#2277728 <https://reviews.llvm.org/D87188#2277728>, @spatel wrote:

> LGTM - I think we should give this a try as-is (with the one-use check still there), see if anything regresses, then ease/remove the use check as a follow-on.

Okay, let's do that.

> As noted, we may need to adjust cost models to account for the size/speed difference that's showing up in unrolling/inlining. That's probably because we assume that an intrinsic is expanded to a single instruction vs. the current cmp+sub+select being 3 instructions?

The default abs cost model already uses icmp+select+sub. Though X86 has custom cost model for the vector variants, which is generally cheaper, so it makes sense that more unrolling is seen there. I expect that this part of the change already happened when the vector intrinsics were switched over recently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87188



More information about the llvm-commits mailing list