[llvm-dev] load instruction erroneously removed by GVN v2

[Daniel Berlin via llvm-dev]

On Wed, Jul 20, 2016 at 9:56 AM, Kevin Choi via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Hello to whom this may concern,
>
> Versioned this as I saw identical title before. I'm compiling a clang
> project where I'm seeing GVN mess up and replace a load with a wrong def
> value.
>


Do you have a c testcase or a .ll file i can actually compile?


> I am using LLVM-3.5, but the problem has been observed upto 3.8.
> To illustrate the problem,
>
> define i32 @main
> scalar.ph:
>     <initialize [80 x i16] %dest>
> ...
> preheader:
> %index=0
> br test, loop1, bb2
> loop1:
>   ... write to %dest in increasing index // ptr-based while loop
>   %ptr++;
>   br test, loop1, bb2
> bb2:
>   %lcssa = phi [%ptr, loop1], [%ptr, preheader]
>   store i16 0, i16* %lcssa !dbg !20094 !tbaa 20030  // write null byte at
> end
>   %76 = getelementptr inbounds [80 x i16]* %dest, i64 0, i64 7, !dbg
> !20095 // load addr of null byte (7th index)
>   %77 = load i16* %76, align 2, !dbg !20095, !tbaa !20010
>   %78 = icmp eq i16 %77, 0, !dbg !20095
>   br i1 %78, label %80, label %79, !dbg !20095
>
> GVN calls processNonLocalLoad() on "%77 = load..." and replaces it with
> init value from scalar.ph.
>
>


> bb2:
> %lcssa = phi [%ptr, loop1], [%ptr, preheader]
> store i16 0, i16* %lcssa, !dbg !20094, !tbaa !20030
> %76 = getelementptr inbounds [80 x i16]* %dest, i64 0, i64 7, !dbg !20095
> br i1 icmp eq (i16 trunc (i128 lshr (i128 bitcast (<8 x i16> <i16 120, i16
> 120, i16 120, i16 120, i16 120, i16 120, i16 120, i16 120> to i128), i128
> 96) to i16), i16 0), label %78, label %77, !dbg !20095  // simplifies to
> "icmp eq (i16 120, i16 0) --> false"
>
> I first suspected problem might be TBAA; invoking clang with
> -fno-strict-aliasing makes the test pass (similarly, opt with -basicaa does
> not make GVN transform the load). When I look at the C/C++ source code, I
> cannot find any type-based aliasing violations from eyeballing.
>

Can you excerpt the actual source code?

>
> I started looking at the aliasing and landed at
> getNonLocalPointerDepFromBB(), in which the worklist algorithm to find
> MemDep reported the result from the init block, ignoring all the kills
> after it. I did see one of the parm was SkipFirstBlock and this appears to
> ignore the store %ptr above the load. Is there a reason why it skips first
> block?
>


Yes. It is looking for *non-local* dependences.  The block it should skip
is bb2, not loop1.
There is another call for local dependences.  If you ask for local
dependences, then non-local ones, as GVN does, there is no point in having
it go and look at the local ones again :)

Look at how GVN calls this:
MemDepResult Dep = MD->getDependency(L);

   // If it is defined in another block, try harder.
   if (Dep.isNonLocal())
     return processNonLocalLoad(L);

(IE get local dependence, if there is none, go looking at non-local
dependences).


The list of non-local dependences it includes should definitely include the
one from block "loop1", and if it is ignoring it, it is most likely thinks
it does not alias for some reason.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160720/5e79a231/attachment.html>