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

[Michael Spencer via llvm-commits]

On Thu, Feb 9, 2017 at 2:02 PM, Davide Italiano <davide at freebsd.org> wrote:

> On Thu, Feb 9, 2017 at 2:01 PM, Davide Italiano <davide at freebsd.org>
> wrote:
> > 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.
> >
>
> Also, I think Eli had some questions about your tests, maybe you can
> address them as a post-commit review (I don't see the changes here).
>
>
Must have missed that. Checking for the load i16 is enough to verify that
they weren't combined.

- Michael Spencer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170209/9116a1b8/attachment.html>