[PATCH] D14899: fixing return value of performScalarPRE() ignored issue

[Daniel Berlin via llvm-commits]

I was about to tell you what you just discovered :)

THe real issue here is that we are inconsistent in what the return values
of these functions mean.
processNonLocalLoad is not returning whether it changed, but whether it
deleted the load.
processLoad in turn is also returning whether it deleted the load.

However, processInstruction then uses this to determine if the function has
changed ...

If you want to make these functions return whether anything changed, i'm
not opposed, but you have to carefully go through the logic separate the
two things it uses the return values for ;)

On Tue, Jan 12, 2016 at 11:24 AM, Wenxiang Qiu via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> vincentqiuuu abandoned this revision.
> vincentqiuuu added a comment.
>
> After studying the test case that failed and GVN.cpp, I found that int the
> function processNonLocalLoad(LoadInst *LI), not taking the return value of
> performScalarPRE(I) into account is actually correct. It's not a bug
> because performScalarPRE(I) is just a step of analyzing when the first
> operand of the LoadInst is a GEP, which means the LoadInst follows a GEP.
> performScalarPRE(I) can return true (the GEP is thus eliminated) whereas
> the LoadInst is not eliminated and thus processNonLocalLoad(LoadInst *LI)
> should return false.
>
>
> http://reviews.llvm.org/D14899
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160112/ba07d1b8/attachment.html>