[PATCH] D132966: [TTI] Allow passing ArrayRef of context instructions (NFC).

Wed Aug 31 09:23:03 PDT 2022

ABataev added a comment.

In D132966#3761409 <https://reviews.llvm.org/D132966#3761409>, @fhahn wrote:

> In D132966#3760480 <https://reviews.llvm.org/D132966#3760480>, @dmgreen wrote:
>
>> This seems to mess up the interface to TTI quite a lot. Are there any other cases than the SLP vectorizer where se would pass a vector of Instructions?
>
> Yeah the new argument is specifically to support SLP's use case. I don't think other passes are in a similar situation at the moment. There's also a version that keeps the logic in SLP: D132872 <https://reviews.llvm.org/D132872>, but @ABataev  argued to have this generally available.

Maybe add a specific function which returns bool if preferable to use FMA instead?

>> The CxtI only has to be a context. It gets a bit fuzzy, but could we just pass the first instruction if it is similar enough to the other instructions in the TreeEntry? It looks like the first item is already passed in at the moment.
>
> I think all instructiosn in a TreeEntry should be very similar in almost all cases (same opcode). But here we need to specifically look at the users to determine if the users of all instructions in the bundle will allow fusion.
>
> Now while spelling this out, maybe we could instead fuse elegible FMUL + FADD/FSUB TreeEntry nodes directly to a single FMULADD/SUB TreeEntry intead of checking for fusion opportunities for the vector version? @ABataev  do you think that would be easily do-able?

Everything is doable, it is just a question of time. Need to adjust the cost somehow, add a flag (probably!) to the node(s) for possible "FMAsation" and change the codegen to emit FMA instead of fmul+fadd/fsub.

>> Also, on a conceptual level - mul's are expensive, addition is relatively cheap. Would it make sense to try and mark the fadd as cheap by looking at the operands? (When I've tried in the past the performance wasn't great).
>
> I think when I tried this a while ago in the other direction it turned out less profitable.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132966