[PATCH] D81728: [InstCombine] Add target-specific inst combining
Sebastian Neubauer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 10 12:22:53 PDT 2020
Flakebi marked an inline comment as done.
Flakebi added inline comments.
================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:540
+ bool instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II,
+ Instruction **ResultI) const;
+ bool simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, IntrinsicInst &II,
----------------
nikic wrote:
> For all three functions, the calling convention seems rather non-idiomatic for InstCombine. Rather than having an `Instruction **` argument and bool result, is there any reason not to have an `Instruction *` return value, with nullptr indicating that the intrinsic couldn't be simplified?
Yes, the function must have the option to return a nullptr and prevent that `visitCallBase` is called or other code is executed after `instCombineIntrinsic`.
So, somehow the caller must be able to see a difference between 'do nothing, just continue execution' and 'return this Instruction*', where the `Instruction*` can also be a nullptr.
The return type could be an `optional<Instruction*>`.
I’ll take a look at your other comments on Monday.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81728/new/
https://reviews.llvm.org/D81728
More information about the cfe-commits
mailing list