[llvm] r294632 - [LoadCombine] Fix combining of loads which span an aliasing store.

[Davide Italiano via llvm-commits]

On Thu, Feb 9, 2017 at 1:46 PM, Michael J. Spencer via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: mspencer
> Date: Thu Feb  9 15:46:49 2017
> New Revision: 294632
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294632&view=rev
> Log:
> [LoadCombine] Fix combining of loads which span an aliasing store.
>
> Fixes PR31517
>
> Differential Revision: https://reviews.llvm.org/D28922
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp
>     llvm/trunk/test/Transforms/LoadCombine/load-combine-aa.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp?rev=294632&r1=294631&r2=294632&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp Thu Feb  9 15:46:49 2017
> @@ -245,13 +245,17 @@ bool LoadCombine::runOnBasicBlock(BasicB
>    bool Combined = false;
>    unsigned Index = 0;
>    for (auto &I : BB) {
> -    if (I.mayThrow() || (I.mayWriteToMemory() && AST.containsUnknown(&I))) {
> +    if (I.mayThrow() || AST.containsUnknown(&I)) {
>        if (combineLoads(LoadMap))
>          Combined = true;
>        LoadMap.clear();
>        AST.clear();
>        continue;
>      }
> +    if (I.mayWriteToMemory()) {
> +      AST.add(&I);
> +      continue;
> +    }
>      LoadInst *LI = dyn_cast<LoadInst>(&I);
>      if (!LI)
>        continue;
>
> Modified: llvm/trunk/test/Transforms/LoadCombine/load-combine-aa.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoadCombine/load-combine-aa.ll?rev=294632&r1=294631&r2=294632&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/LoadCombine/load-combine-aa.ll (original)
> +++ llvm/trunk/test/Transforms/LoadCombine/load-combine-aa.ll Thu Feb  9 15:46:49 2017
> @@ -37,3 +37,24 @@ define i64 @test2(i32* nocapture readonl
>    ret i64 %add
>  }
>
> +%rec11 = type { i16, i16, i16 }
> + at str = global %rec11 { i16 1, i16 2, i16 3 }
> +
> +; PR31517 - Check that loads which span an aliasing store are not combined.
> +define i16 @test3() {
> +; CHECK-LABEL: @test3
> +
> +; CHECK: load i16, i16*
> +; CHECK: store i16
> +; CHECK: ret i16
> +
> +  %_tmp9 = getelementptr %rec11, %rec11* @str, i16 0, i32 1
> +  %_tmp10 = load i16, i16* %_tmp9
> +  %_tmp12 = getelementptr %rec11, %rec11* @str, i16 0, i32 0
> +  store i16 %_tmp10, i16* %_tmp12
> +  %_tmp13 = getelementptr %rec11, %rec11* @str, i16 0, i32 0
> +  %_tmp14 = load i16, i16* %_tmp13
> +  %_tmp15 = icmp eq i16 %_tmp14, 3
> +  %_tmp16 = select i1 %_tmp15, i16 1, i16 0
> +  ret i16 %_tmp16
> +}
>
>

I think it's a good idea to change this tests to not rely on
instcombine anymore.

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare