[PATCH] [InstCombineAddSub] Added the transformation - "sub (or A B) (xor A B) -> (and A B)"
Hal Finkel
hfinkel at anl.gov
Sat Oct 18 21:51:10 PDT 2014
----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: reviews+D5719+public+3088893b38cafcf5 at reviews.llvm.org
> Cc: "ankur29 garg" <ankur29.garg at samsung.com>, "suyog sarda" <suyog.sarda at samsung.com>, llvm-commits at cs.uiuc.edu
> Sent: Saturday, October 18, 2014 11:37:40 PM
> Subject: Re: [PATCH] [InstCombineAddSub] Added the transformation - "sub (or A B) (xor A B) -> (and A B)"
>
> ----- Original Message -----
> > From: "David Majnemer" <david.majnemer at gmail.com>
> > To: "ankur29 garg" <ankur29.garg at samsung.com>,
> > dexonsmith at apple.com, "suyog sarda" <suyog.sarda at samsung.com>,
> > "david
> > majnemer" <david.majnemer at gmail.com>
> > Cc: llvm-commits at cs.uiuc.edu
> > Sent: Saturday, October 18, 2014 4:47:12 AM
> > Subject: Re: [PATCH] [InstCombineAddSub] Added the transformation -
> > "sub (or A B) (xor A B) -> (and A B)"
> >
> > InstCombine is not supposed to handle every possible just because
> > it
> > is theoretically possible case. It's engineered to be a series of
> > engineering tradeoffs. I don't know what the history of the
> > similar
> > transform is in VisitAdd, it's possible that it wasn't added in a
> > principled way.
> >
> > If we were to arbitrarily add patterns, we would eventually make
> > InstCombine cripplingly slow.
>
> To be honest, I think this position is unhelpful. A few thoughts:
>
> - In my experience (derived from my work on an instruction-level
> superoptimizer), it is really hard to predict what patterns will
> occur in practice -- and given knowledge of certain bits from
> switch statements, enums (range metadata), etc. combined with
> LLVM's canonicalization in terms of large integers, and several
> other factors, lead to many patterns that I would consider strange,
> and not-uncommonly in performance-sensitive regions of code.
To add another anecdote, just when I thought I had classified the kinds of patterns that were occurring in practice, I started looking at instrumented code (address sanitizer, etc.), and I learned that I had a lot left to do.
-Hal
>
> - We (obviously) should not engage in premature optimization -- I've
> seen no evidence that matching these kinds of patterns in
> InstCombine contributes a significant portion of its running time.
> And until such evidence is presented, we should not make statements
> about expense (there is no complexity argument here). In any case,
> looking at combinations of three binary operators, are there any
> more than a few hundred that could simplify in any reasonable
> sense? Probably not even that many. I see no reason we should not
> handle them all.
>
> - For simplifications that require certain bits to be known, calls
> into ValueTracking can be expensive, but making duplicate calls
> into ValueTracking is a separate issue that can be fixed if it
> becomes important.
>
> - I feel that, in the past, the real engineering tradeoff was
> slightly different: it used to be difficult to prove the
> transformations correct. As a result, there was a significant
> possibility of bugs being introduced by each of these
> transformations, and it made more sense to limit them. As you know,
> this is no longer anywhere near as true as it was: we now have good
> tools to verify these transformations.
>
> Thanks again,
> Hal
>
> >
> > http://reviews.llvm.org/D5719
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list