<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 2, 2022 at 5:45 PM Roman Divacky via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Nov 02, 2022 at 02:26:15AM -0700, Nikita Popov via llvm-commits wrote:<br>
> <br>
> Author: Nikita Popov<br>
> Date: 2022-11-02T10:23:06+01:00<br>
> New Revision: 7ad3bb527e25eb0a9d147d2e93f9dca605c75688<br>
> <br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/7ad3bb527e25eb0a9d147d2e93f9dca605c75688" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/7ad3bb527e25eb0a9d147d2e93f9dca605c75688</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/7ad3bb527e25eb0a9d147d2e93f9dca605c75688.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/7ad3bb527e25eb0a9d147d2e93f9dca605c75688.diff</a><br>
> <br>
> LOG: Reapply [ValueLattice] Fix getCompare() for undef values<br>
> <br>
> Relative to the previous attempt, this also updates the<br>
> ValueLattice unit tests.<br>
> <br>
> -----<br>
> <br>
> Resolve the TODO about incorrect getCompare() behavior. This can<br>
> be made more precise (e.g. by materializing the undef value and<br>
> performing constant folding on it), but for now just return an<br>
> unknown result to fix the correctness issue.<br>
> <br>
> This should be NFC in terms of user-visible behavior, because the<br>
> only user of this method (SCCP) was already guarding against<br>
> UndefValue results.<br>
> <br>
> Added: <br>
>     <br>
> <br>
> Modified: <br>
>     llvm/include/llvm/Analysis/ValueLattice.h<br>
>     llvm/lib/Transforms/Utils/SCCPSolver.cpp<br>
>     llvm/unittests/Analysis/ValueLatticeTest.cpp<br>
> <br>
> Removed: <br>
>     <br>
> <br>
> <br>
> ################################################################################<br>
> diff  --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h<br>
> index bc6b279e9ed52..7fe45d9f4dc96 100644<br>
> --- a/llvm/include/llvm/Analysis/ValueLattice.h<br>
> +++ b/llvm/include/llvm/Analysis/ValueLattice.h<br>
> @@ -453,8 +453,14 @@ class ValueLatticeElement {<br>
>    /// evaluated.<br>
>    Constant *getCompare(CmpInst::Predicate Pred, Type *Ty,<br>
>                         const ValueLatticeElement &Other) const {<br>
> -    if (isUnknownOrUndef() || Other.isUnknownOrUndef())<br>
> -      return UndefValue::get(Ty);<br>
> +    // Not yet resolved.<br>
> +    if (isUnknown() || Other.isUnknown())<br>
> +      return nullptr;<br>
> +<br>
> +    // TODO: Can be made more precise, but always returning undef would be<br>
> +    // incorrect.<br>
> +    if (isUndef() || isUndef())<br>
> +      return nullptr;<br>
<br>
Hi,<br>
<br>
The above condition looks like a typo. Did you perhaps mean isUndef() || Other.isUndef() ?<br></blockquote><div><br></div><div>Indeed, thanks for spotting! Should be fixed with <a href="https://github.com/llvm/llvm-project/commit/96a74c452728fd330f99394bb25dacecd9325645">https://github.com/llvm/llvm-project/commit/96a74c452728fd330f99394bb25dacecd9325645</a>.</div><div><br></div><div>Regards,<br></div><div>Nikita<br> </div></div></div>