[PATCH] D150316: [AArch64][InstCombine] Don't scalarize for bitselet instructions

Tue May 23 02:23:47 PDT 2023

dmgreen added a reviewer: nikic.
dmgreen added a comment.
Herald added a subscriber: StephenFan.

In D150316#4361813 <https://reviews.llvm.org/D150316#4361813>, @pranavk wrote:

> @dmgreen I think the right solution to this problem is to come up with some sort of cost model here so that these things are handled appropriately. But I would like your or someone's input on this as well. Let me know.

Do you mean to get cheapToScalarize to not introduce more extracts than it removes? As in the FIXME a few lines up? That sounds sensible to me on paper. I looked at it briefly before, there may be some issues getting it working well but if the performance looks OK then that sounds like a good way to go. Instcombine shouldn't really be inserting more instructions and some extracts are not expected to be cheap.

In terms of using the actual costmodel through TTI getInstructionCost, then InstCombine is expected to be a target independent canonicalization pass, and shouldn't use those methods. But hopefully that isn't needed in this case.

Otherwise it might be possible to fix up in the backend, if improving this code looks like it will cause too many other issues.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150316