[PATCH] D122431: Basic support for posix_memalign / __builtin_object_size interaction

[serge via Phabricator via llvm-commits]

serge-sans-paille added inline comments.


================
Comment at: llvm/include/llvm/Analysis/MemoryBuiltins.h:179
+                           const TargetLibraryInfo *TLI, AAResults *AA,
+                           bool MustSucceed);
 
----------------
nikic wrote:
> Make it an optional parameter instead?
That's a choice: I wanted to keep it grouped with `TLI` so that all analyse are kept next to each other.


================
Comment at: llvm/lib/Analysis/MemoryBuiltins.cpp:843
+      if (!Options.AA) {
+        if (SI->getOperand(1) == Load.getOperand(0))
+          return compute(SI->getOperand(0));
----------------
nikic wrote:
> Use `getPointerOperand()` here.
> 
> Also probably need to check `SI->getValueOperand()->getType()->isPointerTy()`, unless compute() handles non-pointer values gracefully.
I *think* this is already guarenteed by the fact that we match against the load pointer operand, which is of type `T**`


================
Comment at: llvm/lib/Analysis/MemoryBuiltins.cpp:831
+  auto Unknown = [this]() {
+    ++ObjectVisitorLoad;
+    return unknown();
----------------
nikic wrote:
> This should probably only be counted once on the top level?
This is used to count the "Number of load instructions with unsolved size and offset", thus the detailed counting.


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

https://reviews.llvm.org/D122431