[PATCH] D81728: [InstCombine] Add target-specific inst combining

Mon Jul 13 03:04:58 PDT 2020

Flakebi marked an inline comment as done.
Flakebi added inline comments.

================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:542
+  bool simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, IntrinsicInst &II,
+                                        APInt DemandedMask, KnownBits &Known,
+                                        bool &KnownBitsComputed,
----------------
nikic wrote:
> `const APInt &DemandedMask`?
I tried to change it it to to `const APInt &DemandedMask` but the x86 simplifyDemandedVectorEltsIntrinsic changes `DemandedMask`, so this function would have to copy it or take a non-const reference.
Looking more into it, `SimplifyAndSetOp` takes `DemandedElts` by value too.
An `APInt` consists of a `uint64_t` and an `unsigned`, so it should be 16 Byte in most cases. Only if the represented int is larger than 64 bit, it comes with an allocation. I guess copying should be fine.
If you think it should be a reference anyway, let me know and I’ll change it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728