This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation



-----Original Message-----
From: Jeff Law [mailto:law@redhat.com] 
Sent: Thursday, August 20, 2015 9:19 PM
To: Ajit Kumar Agarwal; Richard Biener
Cc: GCC Patches; Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation

On 08/20/2015 09:38 AM, Ajit Kumar Agarwal wrote:

>>
>> Bootstrapping with i386 and Microblaze target works fine. No 
>> regression is seen in Deja GNU tests for Microblaze. There are lesser 
>> failures. Mibench/EEMBC benchmarks were run for Microblaze target and 
>> the gain of 9.3% is seen in rgbcmy_lite the EEMBC benchmarks.
>>> What do you mean by there are "lesser failures"?  Are you saying there are cases where path splitting generates incorrect code, or cases where path >>splitting produces code that is less efficient, or something else?
>
> I meant there are more Deja GNU testcases passes with the path splitting changes.
>>Ah, in that case, that's definitely good news!

Thanks. The following testcase testsuite/gcc.dg/tree-ssa/ifc-5.c

void
dct_unquantize_h263_inter_c (short *block, int n, int qscale, int nCoeffs)
{
  int i, level, qmul, qadd;

  qadd = (qscale - 1) | 1;
  qmul = qscale << 1;

  for (i = 0; i <= nCoeffs; i++)
    {
      level = block[i];
      if (level < 0)
        level = level * qmul - qadd;
      else
        level = level * qmul + qadd;
      block[i] = level;
    }
}

The above Loop is a candidate of path splitting as the IF block merges at the latch of the Loop and the path splitting duplicates
The latch of the loop which is the statement block[i] = level into the predecessors THEN and ELSE block.

Due to above path splitting,  the IF conversion is disabled and the above IF-THEN-ELSE is not IF-converted and the test case fails.

There were following review comments from the above patch.

+/* This function performs the feasibility tests for path splitting
> +   to perform. Return false if the feasibility for path splitting
> +   is not done and returns true if the feasibility for path splitting
> +   is done. Following feasibility tests are performed.
> +
> +   1. Return false if the join block has rhs casting for assign
> +      gimple statements.

Comments from Jeff:

>>These seem totally arbitrary.  What's the reason behind each of these 
>>restrictions?  None should be a correctness requirement AFAICT.  

In the above patch I have made a check given in point 1. in the loop latch and the Path splitting is disabled and the IF-conversion
happens and the test case passes.

I have incorporated the above review comments of not doing the above feasibility check of the point 1 and the above testcases goes
For path splitting and due to path splitting the if-cvt is not happening and the test case fails (expecting the pattern Applying if conversion 
To be present). With the above patch given for review and the Feasibility check of cast assign in the latch of the loop as given in point 1
 disables the path splitting  and if-cvt happens and the above test case passes.

Please let me know whether to keep the above feasibility check as given in point 1  or better appropriate changes required for the above 
Test case scenario of path splitting vs IF-conversion.

Thanks & Regards
Ajit


jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]