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

[Nikita Popov via llvm-commits]

On Wed, Nov 2, 2022 at 5:45 PM Roman Divacky via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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() ?
>

Indeed, thanks for spotting! Should be fixed with
https://github.com/llvm/llvm-project/commit/96a74c452728fd330f99394bb25dacecd9325645
.

Regards,
Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221102/a4797680/attachment.html>