This is an archive of the discontinued LLVM Phabricator instance.

[docs] cttz and ctlz return poison, not undef, when argument is 0
Needs ReviewPublic

Authored by pranavk on Dec 29 2018, 1:11 PM.

Diff Detail

Event Timeline

pranavk created this revision.Dec 29 2018, 1:11 PM
lebedev.ri added inline comments.
docs/LangRef.rst
12251–12252

Should defined also be reworded, since it is no longer 'un'defined, but poison?

regehr marked an inline comment as done.Dec 29 2018, 4:10 PM
regehr added inline comments.
docs/LangRef.rst
12251–12252

my view is the "undefined" is a broad term that encompasses undef and poison, so this probably doesn't require rewording

however, is_zero_undef is specifically referring to undef, and should be changed to is_zero_poison (or, perhaps, is_zero_undefined)

As a justification why it doesn't matter whether it's undef or poison, clang emits__builtin_ffs as:
ffs(x) -> x ? cttz(x) + 1 : 0

https://clang.llvm.org/doxygen/CGBuiltin_8cpp_source.html#l01814

So I support this change to poison, since it makes things easier than undef.

nikic added a subscriber: nikic.Dec 30 2018, 2:44 PM

Based on the current documentation of poison values, selecting over poison is still poison by the first rule. I'm assuming that this is an inaccuracy in the documentation and selects are treated the same ways as phis? Otherwise the cited clang code would be affected.

Based on the current documentation of poison values, selecting over poison is still poison by the first rule. I'm assuming that this is an inaccuracy in the documentation and selects are treated the same ways as phis? Otherwise the cited clang code would be affected.

That's a good point. I agree the current documentation of poison is outdated; it misses a rule for select. "select a, b, c" is only poison if a=1 and b is poison or a=0 and c is poison.

Reverse ping.

Note that the poison text for selects was updated after:
rGfaba1d034a
rGd8061456b
( https://bugs.llvm.org/show_bug.cgi?id=20895 )

Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2020, 6:25 AM
sanjoy resigned from this revision.Jan 29 2022, 5:30 PM