[PATCH] D63243: [WIP] Adjust the users of dereferenceable wrt. dereferenceable_globally

[Johannes Doerfert via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/IR/Value.cpp:689
         DerefBytes = CI->getLimitedValue();
+        DerefKnownBefore = LI->getNextNode();
       }
----------------
reames wrote:
> Same point as before, but explained differently in the hopes this makes sense.
> 
> DerefKnownBefore only ever takes one of two values: the first instruction within the function, or the one after the instruction we invoked this function on.  As such, the return value adds no information which can't be easily determined from a single boolean.  
I think I get it know and I hope that means I can convince you we need more than a boolean, at least in the future and if we want to make it easier for the clients.

First, in the future we could handle any subset of the following cases which would make the boolean insufficient:
 - casts
 -  select/phi
 -  gep with constant offsets and dereferenceable base pointer
 - inbound geps with positive offsets w/ or w/o dereferenceabl base pointer

Second, the clients would need to do "more" in the boolean case as they have to check, cast, and advance the pointer value before they can call `maybeFreedInbetween`. Checked, means `isa<Argument>(ptr) || isa<Instruction>(ptr)` as we otherwise have an implicit contract that the bool is only set for instructions and arguments. Casted into the right thing, instruction or argument,  and advanced in case of the former. I'd argue that will more easily to errors or wrong usage.
   



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63243