[llvm] 7ad3bb5 - Reapply [ValueLattice] Fix getCompare() for undef values

[Roman Divacky via llvm-commits]

On Wed, Nov 02, 2022 at 02:26:15AM -0700, Nikita Popov via llvm-commits wrote:
> 
> Author: Nikita Popov
> Date: 2022-11-02T10:23:06+01:00
> New Revision: 7ad3bb527e25eb0a9d147d2e93f9dca605c75688
> 
> URL: https://github.com/llvm/llvm-project/commit/7ad3bb527e25eb0a9d147d2e93f9dca605c75688
> DIFF: https://github.com/llvm/llvm-project/commit/7ad3bb527e25eb0a9d147d2e93f9dca605c75688.diff
> 
> LOG: Reapply [ValueLattice] Fix getCompare() for undef values
> 
> Relative to the previous attempt, this also updates the
> ValueLattice unit tests.
> 
> -----
> 
> Resolve the TODO about incorrect getCompare() behavior. This can
> be made more precise (e.g. by materializing the undef value and
> performing constant folding on it), but for now just return an
> unknown result to fix the correctness issue.
> 
> This should be NFC in terms of user-visible behavior, because the
> only user of this method (SCCP) was already guarding against
> UndefValue results.
> 
> Added: 
>     
> 
> Modified: 
>     llvm/include/llvm/Analysis/ValueLattice.h
>     llvm/lib/Transforms/Utils/SCCPSolver.cpp
>     llvm/unittests/Analysis/ValueLatticeTest.cpp
> 
> Removed: 
>     
> 
> 
> ################################################################################
> diff  --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h
> index bc6b279e9ed52..7fe45d9f4dc96 100644
> --- a/llvm/include/llvm/Analysis/ValueLattice.h
> +++ b/llvm/include/llvm/Analysis/ValueLattice.h
> @@ -453,8 +453,14 @@ class ValueLatticeElement {
>    /// evaluated.
>    Constant *getCompare(CmpInst::Predicate Pred, Type *Ty,
>                         const ValueLatticeElement &Other) const {
> -    if (isUnknownOrUndef() || Other.isUnknownOrUndef())
> -      return UndefValue::get(Ty);
> +    // Not yet resolved.
> +    if (isUnknown() || Other.isUnknown())
> +      return nullptr;
> +
> +    // TODO: Can be made more precise, but always returning undef would be
> +    // incorrect.
> +    if (isUndef() || isUndef())
> +      return nullptr;

Hi,

The above condition looks like a typo. Did you perhaps mean isUndef() || Other.isUndef() ?

Thanks, Roman