[llvm] [llvm] Fix __builtin_object_size interaction between Negative Offset … (PR #111827)
Harald van Dijk via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 24 18:13:29 PDT 2024
hvdijk wrote:
> Indeed. Looks like my previous approach which propagates the constant (negative) offset would have worked in that situation. But it couldn't work with a dynamic negative offset!
You're right, that would handle that one. Can I suggest an alternative though?
Right now, we act on `SizeOffsetAPInt` objects which contain a size (optional) and offset (optional), and the problem with the PHI handling is that we intentionally risk returning an incorrect offset and an incorrect size, on the assumption that we know how the result will be used and the two errors will cancel out. It turns out they do not cancel out in all cases because we do not have a full understanding of how they are used.
What if, instead, we track *three* `APInt`s? What if we track `TotalSize`, `Offset`, `RemainingSize`?
For my original test, we have PHIs where the incoming values resolve to `SizeOffsetAPInt`s of `{0, 0}` and `{16, 16}`. With my idea, they would instead resolve to `{0, 0, 0}` and `{16, 16, 0}`. This can then be correctly combined into `{unknown, unknown, 0}`, which gives us enough information to say that if a subsequently applied offset is negative, we cannot draw any conclusions. At the same time, this fully preserves the `RemainingSize` to allow the currently intended optimisations, which assume positive offsets, to continue to be performed: for any positive offset, only `RemainingSize` needs to be looked at.
It does not quite handle all the cases that your `ConstantOffset` approach handles, but it would, I believe, be easier to understand and reason to be correct, it would be easier to assure ourselves we are not missing any cases.
https://github.com/llvm/llvm-project/pull/111827
More information about the llvm-commits
mailing list