[PATCH] D148698: [NFC] Rename isPointerOffset to getPointerOffsetFrom and move to Value.h

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.
This revision is now accepted and ready to land.

LGTM -- it makes me twitch a little to move more code into the 'Core' library area, but there's already precedent for there being GEP-examining methods there (the offset-stripping methods for example). The overall argument seems alright: "We're performing this pointer-examination operation as part of debug-info updating now, because we're also tracking variables when they're on the stack, thus we need this method in the 'Core' library".



================
Comment at: llvm/include/llvm/IR/Value.h:772
 
+  /// If this ptr is provably equal \p Other plus a constant offset, return
+  /// that offset in bytes. Essentially `ptr this` subtract `ptr Other`.
----------------
jmorse wrote:
> I feel this would be better placed next to the earlier methods to do with constant-offsets and bounds checking -- those are dedicated to GEPs and the like. (i.e., 30 lines up)



================
Comment at: llvm/include/llvm/IR/Value.h:772-775
+  /// If this ptr is provably equal \p Other plus a constant offset, return
+  /// that offset in bytes. Essentially `ptr this` subtract `ptr Other`.
+  std::optional<int64_t> getPointerOffsetFrom(const Value *Other,
+                                              const DataLayout &DL) const;
----------------
I feel this would be better placed next to the earlier methods to do with constant-offsets and bounds checking -- those are dedicated to GEPs and the like. (i.e., 30 lines up)


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

https://reviews.llvm.org/D148698