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

Chandler Carruth chandlerc at gmail.com
Tue Feb 10 18:08:14 PST 2015


Having thought about this, I think this is safe for 3.6. Thanks Hans.

On Tue, Feb 10, 2015 at 5:21 PM, Hans Wennborg <hans at chromium.org> wrote:

> 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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150210/8d8eb58f/attachment.html>


More information about the llvm-commits mailing list