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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
