[llvm-commits] [llvm] r119728 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp test/CodeGen/X86/2010-11-18-SelectOfExtload.ll

[Frits van Bommel]

On Thu, Nov 18, 2010 at 9:05 PM, Duncan Sands <baldrick at free.fr> wrote:
> Author: baldrick
> Date: Thu Nov 18 14:05:18 2010
> New Revision: 119728
>
> URL: http://llvm.org/viewvc/llvm-project?rev=119728&view=rev
> Log:
> The DAGCombiner was threading select over pairs of extending loads even
> if the extension types were not the same.  The result was that if you
> fed a select with sext and zext loads, as in the testcase, then it
> would get turned into a zext (or sext) of the select, which is wrong
> in the cases when it should have been an sext (resp. zext).  Reported
> and diagnosed by Sebastien Deldon.

> +        // If this is an EXTLOAD, the kind of extension must match.
> +        (LLD->getExtensionType() != RLD->getExtensionType() &&
> +         // The only exception is if one of the extensions is anyext.
> +         LLD->getExtensionType() != ISD::EXTLOAD &&
> +         RLD->getExtensionType() != ISD::EXTLOAD) ||

This looked funny to me, so I checked the rest of the function. Later
on, there's this code:

      Load = DAG.getExtLoad(LLD->getExtensionType(), // ...

Shouldn't this be changed to something like

      Load = DAG.getExtLoad((LLD->getExtensionType() != ISD::EXTLOAD ?
LLD->getExtensionType() : RLD->getExtensionType()), // ...

?

It looks to me like it could now still turn e.g. (select %cond (anyext
%a) (sext %b)) into (anyext (select %cond %a %b)) which it probably
shouldn't.