submit [PATCH] SimplifyCFG for code review

[Evan Cheng]

Hi Mei,

I have concerns about the general approach. It's not clear to me this kind of transformation is generally a win on all modern architectures. As an example from your code:

+/// Before:
+///   ......
+///   %cmp10 = fcmp une float %tmp1, %tmp2
+///   br i1 %cmp1, label %if.then, label %lor.rhs
+///
+/// lor.rhs:
+///   ......
+///   %cmp11 = fcmp une float %tmp3, %tmp4
+///   br i1 %cmp11, label %if.then, label %ifend
+///
+/// if.end:  // the merge block
+///   ......
+///
+/// if.then: // has two predecessors, both of them contains conditional branch.
+///   ......
+///   br label %if.end;
+///
+/// After:
+///  ......
+///  %cmp10 = fcmp une float %tmp1, %tmp2
+///  ......
+///  %cmp11 = fcmp une float %tmp3, %tmp4
+///  %cmp12 = or i1 %cmp10, %cmp11    // parallel-or mode.
+///  br i1 %cmp12, label %if.then, label %ifend
+///
+///  if.end:
+///    ......
+/// 
+///  if.then:
+///    ......
+///    br label %if.end;

The pre-transformed code is probably going to run faster than the transformed code on a modern Intel x86 processor. On ARM processors, saving the value of conditional registers and or'ing them is probably also quite expensive. I also suspect the transformation will prohibit the target from some if-conversion opportunities.

Have you thought about having targets providing cost information to the pass? Alternatively this may be done as a codegen pass?

Evan


On Jun 20, 2013, at 4:26 PM, "Ye, Mei" <Mei.Ye at amd.com> wrote:

> Hi Sean
>  
> See the attached new patch, which fixes all issues brought up by you.
> I ran “clang-format” on whole files which makes more diffs.  Hopefully this is OK with you.
> It will be nice if the check-in process can automatically run “clang-format”.
>  
> -Mei
>  
> From: Sean Silva [mailto:silvas at purdue.edu] 
> Sent: Tuesday, June 18, 2013 4:48 PM
> To: Ye, Mei
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: submit [PATCH] SimplifyCFG for code review
>  
>  
>  
> 
> On Tue, Jun 18, 2013 at 4:27 PM, Ye, Mei <Mei.Ye at amd.com> wrote:
> Hi Sean
>  
> There are some considerations that go into this test sample to create an opportunity for the transformation.
> The test case should run *only* the simplifycfg pass (I see now that you have `-O3` in your test case; please remove `-O3`). You can probably just write a simple .ll function with IR that looks like the "Before" case in your documentation comment. 
>  
> Also,
>  
> +; RUN: opt < %s -O3 -simplifycfg -S | grep br | count 2
> +%struct.float2_tag = type { float, float }
>  
> Please use FileCheck for all new tests. Using grep is deprecated.
>  
> -- Sean Silva
> <odc_simplifycfg>_______________________________________________
> 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/20130620/a5e40298/attachment.html>