[PATCH] [InstCombine] Teach how to fold a select into a cttz/ctlz with the 'is_zero_undef' flag cleared.

[Andrea Di Biagio]

On Fri, Jan 9, 2015 at 7:23 PM, Pete Cooper <peter_cooper at apple.com> wrote:
> Hi Andrea
> <snip..>

> So I agree if you do more than speculate a single cttz.  In that case, you’d be hoisting say 2 instructions out, and flattening some control flow.  However, If you are trying to flatten the specific pattern you gave:
>
> define i64 @test1(i64 %A) {
> ; CHECK-LABEL: @test1(
> ; CHECK: [[CTLZ:%[A-Za-z0-9]+]] = call i64 @llvm.ctlz.i64(i64 %A, i1 false)
> ; CHECK-NEXT: select i1 %tobool, i64 64, i64 [[CTLZ]]
> entry:
>   %tobool = icmp eq i64 %A, 0
>   br i1 %tobool, label %cond.end, label %cond.true
>
> cond.true:                                        ; preds = %entry
>   %0 = tail call i64 @llvm.ctlz.i64(i64 %A, i1 true)
>   br label %cond.end
>
> cond.end:                                         ; preds = %entry, %cond.true
>   %cond = phi i64 [ %0, %cond.true ], [ 64, %entry ]
>   ret i64 %cond
> }
>
> then in this case, you’d flatten (speculate) it in SimplifyCFG, turn it in to the inverse intrinsic, and then later in CodeGen you’d generate this control flow again.
>
> I know that seems a bit strange to flatten some control flow only to reintroduce it later, but flattening it might enable some other useful optimizations.

I see what you mean.

So, if I understood it correctly, you suggest to do the following:
 1) teach CodeGenPrepare how to convert a call to cttz/ctlz with
'is_zero_undef' cleared if the cttz/ctlz is not "cheap" to speculate;
 2) move the logic for matching cttz/ctlz into SimplifyCFG (basically,
we always speculate cttz/ctlz; in case, if we see that it was a bad
decision, we can still revert the change in CodeGenPrepare);

I like this approach (although I agree it is a bit convoluted :-) ).
If other people agree with this design, then I will be happy to send
patches on next week to implement 1. and 2.

In the meantime, if ok for you, I plan to update this patch addressing
the comments from David.
This patch would still be valid regardless of whether we change that
design (correct me if I am wrong).

Thanks Pete!
Andrea

> Thanks,
> Pete
>>
>> I hope this clears up any misunderstanding.
>>
>> -Andrea
>>
>>> Thanks,
>>
>>> Pete
>>
>>
>>
>>
>>
>> http://reviews.llvm.org/D6891
>>
>> EMAIL PREFERENCES
>>  http://reviews.llvm.org/settings/panel/emailpreferences/
>>
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits