[llvm] 608ee70 - [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Philip Reames via llvm-commits]

I'm confused by this change.  Your commit message talks about aren't 
interposable symbols, but the code you're removing explicitly contains a 
check for whether the symbol was interprosable, and as far as I can 
tell, only changes behavior for symbols which *are* interprosable.

Can you explain a bit further what's going on here?  I suspect you may 
be fixing a symptom, not a root cause.

Philip

On 2/23/23 11:59, Leonard Chan via llvm-commits wrote:
> Author: Leonard Chan
> Date: 2023-02-23T19:59:25Z
> New Revision: 608ee703e530b00130923417b6663c9f3816f889
>
> URL: https://github.com/llvm/llvm-project/commit/608ee703e530b00130923417b6663c9f3816f889
> DIFF: https://github.com/llvm/llvm-project/commit/608ee703e530b00130923417b6663c9f3816f889.diff
>
> LOG: [SCEV] Ensure SCEV does not replace aliases with their aliasees
>
> Passes in general shouldn't replace an alias with the aliasee (see
> https://reviews.llvm.org/D66606). This can lead to situations where a
> linkonce_odr symbol (which could be interposable if lowered to weak
> linkage) can be replaced with a local aliasee which won't be
> interposable. SVEC does this when the function is invoked by
> FunctionPass Manager -> Loop Pass Manager -> Induction Variable Users in
> the codegen pipeline. This was found in hwasan instrumented code where a
> linonce_odr alias was replaced with its private aliasee.
>
> This fixes the bug descriped at
> https://github.com/llvm/llvm-project/issues/60668.
>
> Differential Revision: https://reviews.llvm.org/D144035
>
> Added:
>      llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
>
> Modified:
>      llvm/lib/Analysis/ScalarEvolution.cpp
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
> index 5c2c12c7d0591..93c9349b06b82 100644
> --- a/llvm/lib/Analysis/ScalarEvolution.cpp
> +++ b/llvm/lib/Analysis/ScalarEvolution.cpp
> @@ -7429,10 +7429,6 @@ ScalarEvolution::getOperandsToCreate(Value *V, SmallVectorImpl<Value *> &Ops) {
>     } else if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
>       return getConstant(CI);
>     else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
> -    if (!GA->isInterposable()) {
> -      Ops.push_back(GA->getAliasee());
> -      return nullptr;
> -    }
>       return getUnknown(V);
>     } else if (!isa<ConstantExpr>(V))
>       return getUnknown(V);
> @@ -7619,7 +7615,7 @@ const SCEV *ScalarEvolution::createSCEV(Value *V) {
>     } else if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
>       return getConstant(CI);
>     else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(V))
> -    return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
> +    return getUnknown(V);
>     else if (!isa<ConstantExpr>(V))
>       return getUnknown(V);
>   
>
> diff  --git a/llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll b/llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
> new file mode 100644
> index 0000000000000..d4d0b7afbdbc8
> --- /dev/null
> +++ b/llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
> @@ -0,0 +1,19 @@
> +; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
> +; RUN: opt -passes='print<scalar-evolution>' -disable-output %s 2>&1 | FileCheck %s
> +
> +target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
> +target triple = "aarch64-unknown-linux-gnu"
> +
> + at glob.private = private constant [32 x i32] zeroinitializer
> + at glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint (ptr @glob.private to i64), i64 1234) to ptr)
> +
> +define hidden void @func(i64 %idx) local_unnamed_addr {
> +; CHECK-LABEL: 'func'
> +; CHECK-NEXT:  Classifying expressions for: @func
> +; CHECK-NEXT:    %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
> +; CHECK-NEXT:    --> ((4 * %idx) + @glob) U: [0,-1) S: [-9223372036854775808,9223372036854775807)
> +; CHECK-NEXT:  Determining loop execution counts for: @func
> +;
> +  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
> +  ret void
> +}
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits