[llvm] r228656 - [x86] Fix PR22524: the DAG combiner was incorrectly handling illegal

[Hans Wennborg]

On Mon, Feb 9, 2015 at 7:00 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
> Thanks for the quick fix!
>
>> On 2015-Feb-09, at 18:25, Chandler Carruth <chandlerc at gmail.com> wrote:
>>
>> Author: chandlerc
>> Date: Mon Feb  9 20:25:56 2015
>> New Revision: 228656
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=228656&view=rev
>> Log:
>> [x86] Fix PR22524: the DAG combiner was incorrectly handling illegal
>> nodes when folding bitcasts of constants.
>>
>> We can't fold things and then check after-the-fact whether it was legal.
>> Once we have formed the DAG node, arbitrary other nodes may have been
>> collapsed to it. There is no easy way to go back. Instead, we need to
>> test for the specific folding cases we're interested in and ensure those
>> are legal first.
>>
>> This could in theory make this less powerful for bitcasting from an
>> integer to some vector type, but AFAICT, that can't actually happen in
>> the SDAG so its fine. Now, we *only* whitelist specific int->fp and
>> fp->int bitcasts for post-legalization folding. I've added the test case
>> from the PR.
>>
>> (Also as a note, this does not appear to be in 3.6, no backport needed)
>
> @Hans: I think Chandler was relying on my bisection from the PR here, but
> I had only bisected the opt+llc.ll case.  (I first tried to bisect the
> llc.ll case, but couldn't find a passing revision.)
>
> I've just checked the llc.ll case on 3.6 (well, slightly old r228165) and
> it *does* repro, and cherry-picking this commit fixes it.
>
> Probably not *urgent*, since -instcombine isn't exposing it, but still
> seems like a good fix.

I'm okay with merging this if Chandler thinks the patch is safe.

 - Hans