[llvm-dev] [llvm-reduce] Reduction to undef/poison/null?
Arthur Eubanks via llvm-dev
David Blaikie via llvm-dev
I /guess/ undef because it's more vague/shows the value doesn't matter to some degree?
(might be worth CC'ing some of the other folks who've contributed patches/implemented llvm-reduce - not everyone reads llvm-dev or does so frequently enough to see relevant threads in a timely manner)
Currently in llvm-reduce when we try to remove instructions, we'll RAUW them with undef. But it seems nicer to have the final IR use null values (e.g. 0, zeroinitializer, etc) rather than undef. Should we attempt to use null values? Or why undef over poison?
_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Florian Hahn via llvm-dev
> On 30 Aug 2021, at 19:59, David Blaikie via llvm-dev <llvm...@lists.llvm.org> wrote:
>
> Nicer because it's less likely to introduce new UB? Or some other reason?
>
Using undef/poison is problematic, because there are multiple ways this could cause new UB (e.g. branch on undef, passing poison/undef to a function with a noundef argument).
I’m not sure if using zero will work well in certain cases, because it can introduce UB as well (e.g. load from null, passing as nonnull argument).
I think ideally we would have a way to materialise values we know nothing about, but are not undef. Perhaps we could add some oracle function, but that would come with its own drawbacks.
Cheers,
Florian
Roman Lebedev via llvm-dev
but i don't think this question matters for the patch at hand,
it should just stick to the current practice of using undef.
Roman.
Nuno Lopes via llvm-dev
What I usually end up doing when reducing by hand is to introduce a new function parameter to replace a deleted instruction, e.g.:
define @f(i32 %x) {
%y = add i32 %x, ...
...
}
=>
define @f(i32 %x, i32 %y) {
<remove instr %y>
...
}
Though not all users will want this sort of reduction..
I agree with what other said about undef/poison: avoid them at all costs when reducing. It will just expose some other bug regarding handling of undef in optimizers instead of the bug you wanted to track down :)
Nuno
Johannes Doerfert via llvm-dev
> I've been thinking we should be using `freeze poison`,
> but i don't think this question matters for the patch at hand,
> it should just stick to the current practice of using undef.
I like freeze poison. It conveys the idea w/o making things UB all the time.
It basically is an oracle w/o the side effects.
FWIW, when I ported tests to the Attributor, e.g., from
ArgumentPromotion or IPSCCP,
I had to manually remove all the UB that made the test meaningless
first. In general,
tests that contain statically provable UB are less likely to be
meaningful over time
and/or be reusable.
~ Johannes
> Roman.
>
> On Mon, Aug 30, 2021 at 9:14 PM Florian Hahn via llvm-dev
> <llvm...@lists.llvm.org> wrote:
>>
>>
>>> On 30 Aug 2021, at 19:59, David Blaikie via llvm-dev <llvm...@lists.llvm.org> wrote:
>>>
>>> Nicer because it's less likely to introduce new UB? Or some other reason?
>>>
>> Using undef/poison is problematic, because there are multiple ways this could cause new UB (e.g. branch on undef, passing poison/undef to a function with a noundef argument).
>>
>> I’m not sure if using zero will work well in certain cases, because it can introduce UB as well (e.g. load from null, passing as nonnull argument).
>>
>> I think ideally we would have a way to materialise values we know nothing about, but are not undef. Perhaps we could add some oracle function, but that would come with its own drawbacks.
>>
>> Cheers,
>> Florian
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm...@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> llvm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
--
──────────────────────
∽ Johannes Doerfert ∽
∽ Pronouns: he/him ∽
∽ Researcher @ ANL ∽
Arthur Eubanks via llvm-dev
John Regehr via llvm-dev
has a few other nice improvements
https://github.com/regehr/llvm-project/tree/new-llvm-reduce-pass
I haven't yet done the "avoid undef/poison at all costs" thing, but will
get around to it sometime hopefully.
John
On 8/30/21 12:26 PM, Nuno Lopes via llvm-dev wrote:
> What I usually end up doing when reducing by hand is to introduce a new
> function parameter to replace a deleted instruction, e.g.:
Johannes Doerfert via llvm-dev
On 8/30/21 1:36 PM, Arthur Eubanks wrote:
> I frequently use llvm-reduce just to minimize a crash caused by some change
> and present that to the author of a change to look at. I don't think that
> having tons of freeze poisons in a repro file is nice to work with. If a
> crash repros with a `0` as opposed to a `freeze poison` the `0` seems much
> more appealing to present.
>
> We could add a flag to reduce to the various options here if people have
> different needs.
Yeah, maybe. I think it's clear undef is often not the best choice.
Chris Lattner via llvm-dev
> On Aug 31, 2021, at 9:04 AM, Johannes Doerfert via llvm-dev <llvm...@lists.llvm.org> wrote:
>
>
> On 8/30/21 1:36 PM, Arthur Eubanks wrote:
>> I frequently use llvm-reduce just to minimize a crash caused by some change
>> and present that to the author of a change to look at. I don't think that
>> having tons of freeze poisons in a repro file is nice to work with. If a
>> crash repros with a `0` as opposed to a `freeze poison` the `0` seems much
>> more appealing to present.
>>
>> We could add a flag to reduce to the various options here if people have
>> different needs.
>
> Yeah, maybe. I think it's clear undef is often not the best choice.
I agree with the meta point here - use of undef will make the behavior far less predictable, and therefore make reduction more annoying. I think it makes a lot of sense to use a deterministic zero value instead of undef.
Is there any _disadvantage_ to doing that?
-Chris