[PATCH] [InstCombineAddSub] Added the transformation - "sub (or A B) (xor A B) -> (and A B)"

[Hal Finkel]

----- 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.

 - 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