[PATCH] D76459: [SCCP] Use SimplifyBinOp for non-integer constant/expressions & overdef.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D76459#1939662 <https://reviews.llvm.org/D76459#1939662>, @efriedma wrote:

> > By just passing a context with the DL, SimplifyBinOp should not try to get additional information from looking at definitions.
>
> What context is legal to use here?


I think  SimplifyBinOp is not allowed to look at anything besides the values we pass in. In particular, looking at the definitions of the operands could potentially lead to it making assumption about undef values, which might lead to contradictions with the view SCCP has. I think this would lead to correctness issues similar to what NewGVN currently suffers from.

I initially thought passing in a context without DT & CxtI would be enough to ensure that, but I had another look and now I am not too sure. It seems like the SImplify* functions at least pass to operands to helpers from ValueTracking, which in turn may look at the operands of the operand, if it is an instruction. Not sure how to best prevent that now. One way would be to restrict that behavior via a new flag. Or create some kind of freestanding value that does not leak any information. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76459