[llvm-commits] [Review] GVN Patch

[Eli Friedman]

On Fri, Oct 19, 2012 at 2:20 PM, Bill Wendling <wendling at apple.com> wrote:
> For those of you wondering why I was so interested in dominance, there is a situation where GVN is barfing because it goes into an infinite loop. The problem is that it's trying to process a non-local load which has a dependency in a block that's unreachable from the entry point. It gets into an infinite loop because the GEP it is querying is of this form:
>
>         %xx = getelementptr %some.type %xx ...
>
> which I'm told is perfectly valid within unreachable blocks.
>
> Below (inlined because it's so small) is my fix for this. It at least allows the code to compile to completion. I haven't worked with GVN much before, so I don't know if this has any non-local effects.
>
> Please review this and let me know if it's okay to commit. (And I will commit a testcase with this once it's been approved.)
>
> Thanks.
>
> -bw
>
> diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
> index e67d8af..5411786 100644
> --- a/lib/Transforms/Scalar/GVN.cpp
> +++ b/lib/Transforms/Scalar/GVN.cpp
> @@ -1368,6 +1368,8 @@ bool GVN::processNonLocalLoad(LoadInst *LI) {
>      BasicBlock *DepBB = Deps[i].getBB();
>      MemDepResult DepInfo = Deps[i].getResult();
>
> +    if (!DT->isReachableFromEntry(DepBB)) continue;
> +
>      if (!DepInfo.isDef() && !DepInfo.isClobber()) {
>        UnavailableBlocks.push_back(DepBB);
>        continue;

This isn't wrong, exactly... but the check is later than it should be:
memdep shouldn't be returning results for unreachable blocks in the
first place.

-Eli