[PATCH] D84792: [InstSimplify/NewGVN] Add option to control the use of undef.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

> Is the goal here that it's legal to replace the output value of SimplifyInstruction with the input value, in addition to the normal replacement of the input value with the output value? Or do we specifically care about literal UndefValue constants somehow, as opposed to values which are undef? If we had a PoisonValue constant, would we also need to exclude it?

I think the only thing we need to prevent is that InstSimplify assumes a specific value for (or any kind of constraint on) an undef value. We can still allow other refinement, and I don't think poison values are problematic in this context either, because they don't have a consistency requirement (we don't have to pick one particular value for the poison -- and generally can't).

> Do we also need to restrict transforms like "X & 0 = 0"?

Same here, this should not be a problem. Even if X is undef, this transform is valid for any choice of value for X.

> For example, given icmp eq X, Y, if X and Y are both UndefValue, comparison X == Y will yield true.

Hm, this is a tricky case. I agree that doing this fold is not right, but it also seems hard to really enforce, because of how many value comparisons there are (and not all of them are problematic). I think it may be better to ignore this particular case until we have evidence that it can cause issues in practice (the cases where this could occur are a lot more narrow, because at the very least it involves two undef values from distinct sources).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84792