[PATCH] D71799: [Attributor] AAUndefinedBehavior: Check for branches on undef value.

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added a comment.

In D71799#1796066 <https://reviews.llvm.org/D71799#1796066>, @jdoerfert wrote:

> In D71799#1795961 <https://reviews.llvm.org/D71799#1795961>, @baziotis wrote:
>
> > In D71799#1795803 <https://reviews.llvm.org/D71799#1795803>, @jdoerfert wrote:
> >
> > > I'd wait wrt. AAValueSimplify.
> >
> >
> > I guess you meant "wait to be committed" (and not wait as to not use it in the memory accessing functions yet).
>
>
> Yes. Also wrt. modifying AAVAlueSimply further. Using it is not a problem.


Ok, noted.

> 
> 
>>> I'd do the following two first:
>>> 
>>> 1. UnreachableInst is UB (though no need to replace it in the manifest.
>>> 2. Given an instruction, determine if that instruction or one later that is known to be executed is known to cause UB. We then hook that up to the AAIsDead.
>> 
>> Regarding 2), would it suffice to: Go through the next instructions and follow (alive) branches either by taking unconditional branches, or branches that have known value (e.g. using `AAValueSimplify`) `true`.
>>  Note: Walk them in a DFS kind of way until we find a UB instruction (or none at all).
> 
> You want to use `MustBeExecutedContextExplorer`. You get it like this:
> 
>   MustBeExecutedContextExplorer &Explorer =
>       A.getInfoCache().getMustBeExecutedContextExplorer();
> 
> 
> You iterate over it like you would with a container like this:
> 
>   for (MustBeExecutedIterator &It : Explorer.range(Instruction))
> 
> 
> 
> 
>  ---
> 
> There was a patch by @uenoku to deal with conditional and the merging of states when we are exploring but I don't see it in-tree and I forgot which one it was.
>  Nevertheless, it should immediately work for straight line code and code that is in "some merge block" after a conditional.

Aha, ok, thank you! I'll try it in the next revision.

> @uenoku Can you commit this for @baziotis? (https://llvm.org/docs/DeveloperPolicy.html#commit-messages)

Sorry for being late, I would like to have already updated a diff so that @uenoku can commit it but I had some problems with compilation of LLVM when I pulled the last changes.


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

https://reviews.llvm.org/D71799