[PATCH] D78503: RFC: [ValueTracking] Let analyses assume a value cannot be partially poison

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

This patch now only removes the `Full` part from the comments and names? If so, I think that makes sense. Let someone else give a second opinion though.



================
Comment at: llvm/lib/Transforms/Instrumentation/PoisonChecking.cpp:347
    - and/or - It would seem reasonable for poison to propagate from both
-     arguments, but LangRef doesn't state and propagatesFullPoison doesn't
-     include these two.
+     arguments, but LangRef doesn't state.
    - all binary ops w/vector operands - The likely interpretation would be that
----------------
aqjune wrote:
> aqjune wrote:
> > jdoerfert wrote:
> > > Is this true? I mean, don't you check for binary operands above?
> > It was my misunderstanding about the code, updated.
> BTW, what do you think about making this explicit in LangRef? I can make a patch for this.
> If and/or blocks poison propagation, removal of trivial and/ors becomes invalid, because it makes the resulting program more poisonous.
> 
> 
That's a good question. I guess we could either say that bits you "overwrite" with and/or are not poison anymore but I am unsure that is what we want. The problem is that various operations could be used to fix bits and we should handle them all the same (x = y - y, x = y * 0, ...).

That said, we should have a discussion (= ask others).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78503