[PATCH] D75120: [ValueLattice] Add new state for undef constants.

Wed Mar 11 15:56:53 PDT 2020

fhahn marked 2 inline comments as done.
fhahn added inline comments.

================
Comment at: llvm/include/llvm/Analysis/ValueLattice.h:137
+    if (isa<UndefValue>(C))
+      Res.markUndefined();
+    else
----------------
efriedma wrote:
> fhahn wrote:
> > efriedma wrote:
> > > I'm not sure I understand how getNot(undef) is supposed to work.
> > I think it should be fine to assume `x != undef => undef`. Alive seems to agree. Granted, it is probably not very useful. I tried to replace it with an assert ruling out undef here, but we hit this case in practice in LVI via JumpThreading.
> If you prove x != undef, x can't be any specific value... including undef, I think.  So actually you could just treat it as unknown, I think?
Looking at the uses in LVI, it seems like it is used for information coming from things like `%x = icmp ne %a, CONST; %y = select %x, %a, 10` and LVI would use `getNot(CONST)` and use that to mark %y as `!= CONST` if `10 != Const` . (https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/LazyValueInfo.cpp#L953) 

I'm not sure if that allows saying that `%a` cannot be any specific value including undef, if `CONST` is undef. Couldn't the use in `%a` be any value, by choosing something like `%a+1` for undef in the icmp? 

Because IIUC we can only chose a single concrete value for the undef in the icmp per execution. 

Another way to think about it might be a constant range with a single value missing. The missing value can be arbitrarily chosen per use/execution. I think what's allowed and dis-allowed would heavily depend on how it is used exactly. Maybe it would be best to start out with overdefined in this patch and refine it if required later on?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75120