[PATCH] D110245: [ConstantFolding] Fold ptrtoint(gep i8 null, x) -> x

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D110245#3017557 <https://reviews.llvm.org/D110245#3017557>, @arichardson wrote:

> In D110245#3017554 <https://reviews.llvm.org/D110245#3017554>, @lebedev.ri wrote:
>
>> In D110245#3017547 <https://reviews.llvm.org/D110245#3017547>, @arichardson wrote:
>>
>>> In D110245#3017539 <https://reviews.llvm.org/D110245#3017539>, @lebedev.ri wrote:
>>>
>>>> If there isn't a fold `gep inbounds null, ???  -->  bitcast(null)` it might be interesting to add one though.
>>>
>>> I tried adding a `(gep inbounds null, nonzero) -> poison` fold, but that broke lots of tests. `(gep inbounds null, ..) -> null` is a bit safer but will break `&((TYPE *)0)->MEMBER)` __builtin_offsetof fallbacks since they are constant-folded to null before UBSAN can handle it.
>>
>> I personally find this approach problematic for forward progress.
>> https://alive2.llvm.org/ce/z/swyJHv <- `gep inbounds null, ???  -->  null` is always fine
>> https://alive2.llvm.org/ce/z/fQUjCY <- `(gep inbounds null, nonzero)  -->  undef` is also fine
>> So anything that breaks was broken already. In case of tests, they probably just need to be updated.
>
> I would like to add this fold, but what about @nikic's comment about having a defined null pointer? I believe in that case the whole address space is inbounds of null?

Yep, these two folds i linked require `!nullPointerIsDefined`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110245