[llvm-commits] [PATCH] InstCombine enhancement

[Shuxin Yang]

Hi, Eli:

   Thank you for your feedback. See the following interleaving text.

On 11/26/12 1:51 PM, Eli Friedman wrote:
> On Mon, Nov 26, 2012 at 1:32 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>> Hi, dear all:
>>
>>       This change is trying to catch this optimization opportunity (one of
>> the two defects reported in rdar://12329730):
>>
>>     -----------------------------------------------------
>>            ((X^C1) >> C2) ^ C3  => (X>>C2) ^ ((C1>>C2)^C3)
>>       where the subexpression "X ^ C1" has more than one uses, and
>>       "(X^C1) >> C2" has single use.
>>    ------------------------------------------------------
>>
>>      If the "X ^ C1" has only one use,  InstCombine will distribute the ">>"
>> operation
>> in the "X^C1 >> C2", and the entire expression would be eventually
>> optimized;
>> however, if "X ^ C1" has multiple uses,  InstCombine would give up.
> For new code, please use the matchers defined in
> llvm/Support/PatternMatch.h (m_Xor etc.), which tend to be more
> compact than dyn_cast for this sort of matching.
>
> Are there any cases where this transformation could make us generate
> worse code?  If so, how likely do you think it is to happen in
> practice?
It is difficult to envision a case where this change has negative 
impact.  Apparently,
the expression tree/DAG is shorter than it was, and this xform doesn't 
increase
the # of instruction.

>
> This seems to be a general pattern which could be applied to many
> operations (at least "&", "|", "+", "-").  It also seems to apply to
> other shift operations (shl, ashr).  Which of those cases do we handle
> already?  How hard would it be to generalize this transformation?
>
>
If the sub-expression has only one use, as far as I can tell, the 
InstCombine already does a great
job even on a contrived tricky expressions.

However, if the sub-expr has multiple use, it simply give up.

I guess my change is one of the few (if not none) cases, where 
InstCombine can take a glimpse
to the multi-use sub-expression to see if there are opportunities.

Yes, there are other opportunities exposed by taking a quick look into 
multi-use-subexpr;
we have to take it one step at a time:-).

Thanks
Shuxin