[PATCH] D108903: [llvm-reduce] Add reduce operands pass

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 7 15:56:40 PDT 2021


aeubanks added a comment.

In D108903#2988173 <https://reviews.llvm.org/D108903#2988173>, @lebedev.ri wrote:

> In D108903#2988165 <https://reviews.llvm.org/D108903#2988165>, @aeubanks wrote:
>
>> In D108903#2988164 <https://reviews.llvm.org/D108903#2988164>, @swamulism wrote:
>>
>>> In D108903#2988082 <https://reviews.llvm.org/D108903#2988082>, @aeubanks wrote:
>>>
>>>> Sure I suppose we can split up this patch
>>>> @swamulism can you do that?
>>>
>>> Sure I can add `Constant::hasNullValue` and remove `ReduceOperands::typeIsNullable` in a separate patch
>>
>> I meant the part about reducing to undef vs null. `Constant::hasNullValue()` should be in this patch
>
> Also no, that can be separated - place it before `Constant::getNullValue()` / base it on that function,
> and change `Constant::getNullValue()` to first assert that said new function says there is such a constant.
>
> But, i'm still not sold on `null` everywhere. E.g. div-by-zero is ub, `gep inbounds` of `null` w/ non-zero offset is null, etc.

Isn't it the same for undef? Since we could arbitrarily decide to replace all undefs with null. Using undef should always be at least the same amount of ub as using null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108903



More information about the llvm-commits mailing list