Patchwork Resubmit/ping: peephole2 vs cond-exec vs df

login
register
mail settings
Submitter H.J. Lu
Date June 30, 2010, 4:56 a.m.
Message ID <AANLkTinM-3HwJDLNo7BBRIXELRJf-I5VMOTUSbaitZzX@mail.gmail.com>
Download mbox | patch
Permalink /patch/57375/
State New
Headers show

Comments

H.J. Lu - June 30, 2010, 4:56 a.m.
On Tue, Jun 29, 2010 at 8:11 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jun 29, 2010 at 7:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Jun 29, 2010 at 5:43 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Tue, Jun 29, 2010 at 5:41 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>> On Tue, Jun 29, 2010 at 6:22 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>>>>> No, those parts of the buffer that weren't part of the match remain
>>>>> unaffected, we keep both the insns and their life information.  We only
>>>>> rebuild life for the new insns produced by the match.
>>>>>
>>>>> Here's a new version with a few more comments and a few remnants of old
>>>>> code removed.  I've also removed some dead code found in genrecog.c (got
>>>>> sidetracked today into debugging the current peephole2 code again...);
>>>>> this was left in after your r34208 patch.
>>>>
>>>> I think this causes a bootstrap failure on x86_64-linux-gnu:
>>>> /home/apinski/src/gcc-fsf/local//gcc/gcc/coverage.c:151:1: error:
>>>> unrecognizable insn:
>>>> (insn 25 7 26 2
>>>> /home/apinski/src/gcc-fsf/local//gcc/gcc/coverage.c:150 (set (reg:DI 1
>>>> dx)
>>>>        (mem/s:SI (plus:DI (reg/v/f:DI 5 di [orig:64 of ] [64])
>>>>                (const_int 4 [0x4])) [15 entry_2->ctr+0 S4 A32])) -1 (nil))
>>>> /home/apinski/src/gcc-fsf/local//gcc/gcc/coverage.c:151:1: internal
>>>> compiler error: in extract_insn, at recog.c:2127
>>>
>>> +  [(match_scratch:SI 5 "r")
>>>
>>> I think the :SI part is incorrect, we need a DI mode on x86_64 rather
>>> than a SImode.
>>>
>>
>> Like this?
>>
>> --
>> H.J.
>> ---
>> Index: gcc/config/i386/i386.md
>> ===================================================================
>> --- gcc/config/i386/i386.md     (revision 161586)
>> +++ gcc/config/i386/i386.md     (working copy)
>> @@ -17558,7 +17558,7 @@
>>  ;;  leal    (%edx,%eax,4), %eax
>>
>>  (define_peephole2
>> -  [(match_scratch:SI 5 "r")
>> +  [(match_scratch:P 5 "r")
>>    (parallel [(set (match_operand 0 "register_operand" "")
>>                   (ashift (match_operand 1 "register_operand" "")
>>                           (match_operand 2 "const_int_operand" "")))
>>
>
> It doesn't work.
>
> --
> H.J.
>

This seems to work.
Bernd Schmidt - June 30, 2010, 8:50 a.m.
On 06/30/2010 06:56 AM, H.J. Lu wrote:

> This seems to work.

Please commit then.  Sorry about the breakage, I guess one has to test
changes to i386.md on both targets.


Bernd
Richard Guenther - June 30, 2010, 9:48 a.m.
On Wed, Jun 30, 2010 at 10:50 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 06/30/2010 06:56 AM, H.J. Lu wrote:
>
>> This seems to work.
>
> Please commit then.  Sorry about the breakage, I guess one has to test
> changes to i386.md on both targets.

I have committed it.  Bootstrap continues until libjava which still
fails for me running out of virtual memory (16GB ulimit):

virtual memory exhausted: Cannot allocate memory
make[3]: *** [gnu/javax/imageio/jpeg.lo] Error 1
make[3]: *** Waiting for unfinished jobs....
virtual memory exhausted: Cannot allocate memory
make[3]: *** [javax/swing/text.lo] Error 1

Richard.

>
> Bernd
>
Bernd Schmidt - June 30, 2010, 9:49 a.m.
On 06/30/2010 11:48 AM, Richard Guenther wrote:
> On Wed, Jun 30, 2010 at 10:50 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 06/30/2010 06:56 AM, H.J. Lu wrote:
>>
>>> This seems to work.
>>
>> Please commit then.  Sorry about the breakage, I guess one has to test
>> changes to i386.md on both targets.
> 
> I have committed it.  Bootstrap continues until libjava which still
> fails for me running out of virtual memory (16GB ulimit):

Is that new with the peephole2 patch?


Bernd
Richard Guenther - June 30, 2010, 9:52 a.m.
On Wed, Jun 30, 2010 at 11:49 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 06/30/2010 11:48 AM, Richard Guenther wrote:
>> On Wed, Jun 30, 2010 at 10:50 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>>> On 06/30/2010 06:56 AM, H.J. Lu wrote:
>>>
>>>> This seems to work.
>>>
>>> Please commit then.  Sorry about the breakage, I guess one has to test
>>> changes to i386.md on both targets.
>>
>> I have committed it.  Bootstrap continues until libjava which still
>> fails for me running out of virtual memory (16GB ulimit):
>
> Is that new with the peephole2 patch?

No.  It is in fact

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44722

Richard.

>
> Bernd
>
Richard Guenther - June 30, 2010, 10 a.m.
On Wed, Jun 30, 2010 at 11:52 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jun 30, 2010 at 11:49 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 06/30/2010 11:48 AM, Richard Guenther wrote:
>>> On Wed, Jun 30, 2010 at 10:50 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>>>> On 06/30/2010 06:56 AM, H.J. Lu wrote:
>>>>
>>>>> This seems to work.
>>>>
>>>> Please commit then.  Sorry about the breakage, I guess one has to test
>>>> changes to i386.md on both targets.
>>>
>>> I have committed it.  Bootstrap continues until libjava which still
>>> fails for me running out of virtual memory (16GB ulimit):
>>
>> Is that new with the peephole2 patch?
>
> No.  It is in fact
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44722

We're peepholing in circles after your patch.  The peephole2
dump isn't very informative - how can I see which peephole2s
matched?

Richard.

> Richard.
>
>>
>> Bernd
>>
>
Bernd Schmidt - June 30, 2010, 10:19 a.m.
On 06/30/2010 12:00 PM, Richard Guenther wrote:

> We're peepholing in circles after your patch.  The peephole2
> dump isn't very informative - how can I see which peephole2s
> matched?

Not easily.  Maybe breakpoints on all the gen_peephole2_ functions; we
should probably add that to the dumps.

Seems to be something involving FIX insns, I suspect

;; Shorten x87->SSE reload sequences of fix_trunc?f?i_sse patterns.
(define_peephole2
  [(set (match_operand:MODEF 0 "register_operand" "")
	(match_operand:MODEF 1 "memory_operand" ""))
   (set (match_operand:SSEMODEI24 2 "register_operand" "")
	(fix:SSEMODEI24 (match_dup 0)))]
  "TARGET_SHORTEN_X87_SSE
   && peep2_reg_dead_p (2, operands[0])"
  [(set (match_dup 2) (fix:SSEMODEI24 (match_dup 1)))]
  "")

and

(define_peephole2
  [(match_scratch:SF 2 "x")
   (set (match_operand:SSEMODEI24 0 "register_operand" "")
	(fix:SSEMODEI24 (match_operand:SF 1 "memory_operand" "")))]
  "TARGET_AVOID_VECTOR_DECODE && optimize_insn_for_speed_p ()"
  [(set (match_dup 2) (match_dup 1))
   (set (match_dup 0) (fix:SSEMODEI24 (match_dup 2)))]
  "")

are a loop.


Bernd
Richard Guenther - June 30, 2010, 10:31 a.m.
On Wed, Jun 30, 2010 at 12:19 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 06/30/2010 12:00 PM, Richard Guenther wrote:
>
>> We're peepholing in circles after your patch.  The peephole2
>> dump isn't very informative - how can I see which peephole2s
>> matched?
>
> Not easily.  Maybe breakpoints on all the gen_peephole2_ functions; we
> should probably add that to the dumps.
>
> Seems to be something involving FIX insns, I suspect
>
> ;; Shorten x87->SSE reload sequences of fix_trunc?f?i_sse patterns.
> (define_peephole2
>  [(set (match_operand:MODEF 0 "register_operand" "")
>        (match_operand:MODEF 1 "memory_operand" ""))
>   (set (match_operand:SSEMODEI24 2 "register_operand" "")
>        (fix:SSEMODEI24 (match_dup 0)))]
>  "TARGET_SHORTEN_X87_SSE
>   && peep2_reg_dead_p (2, operands[0])"
>  [(set (match_dup 2) (fix:SSEMODEI24 (match_dup 1)))]
>  "")

I suppose adding && !TARGET_AVOID_VECTOR_DECODE
would fix it.  Testing that.

> and
>
> (define_peephole2
>  [(match_scratch:SF 2 "x")
>   (set (match_operand:SSEMODEI24 0 "register_operand" "")
>        (fix:SSEMODEI24 (match_operand:SF 1 "memory_operand" "")))]
>  "TARGET_AVOID_VECTOR_DECODE && optimize_insn_for_speed_p ()"
>  [(set (match_dup 2) (match_dup 1))
>   (set (match_dup 0) (fix:SSEMODEI24 (match_dup 2)))]
>  "")
>
> are a loop.
>
>
> Bernd
>
Bernd Schmidt - June 30, 2010, 10:41 a.m.
On 06/30/2010 12:31 PM, Richard Guenther wrote:
> On Wed, Jun 30, 2010 at 12:19 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 06/30/2010 12:00 PM, Richard Guenther wrote:
>>
>>> We're peepholing in circles after your patch.  The peephole2
>>> dump isn't very informative - how can I see which peephole2s
>>> matched?
>>
>> Not easily.  Maybe breakpoints on all the gen_peephole2_ functions; we
>> should probably add that to the dumps.
>>
>> Seems to be something involving FIX insns, I suspect
>>
>> ;; Shorten x87->SSE reload sequences of fix_trunc?f?i_sse patterns.
>> (define_peephole2
>>  [(set (match_operand:MODEF 0 "register_operand" "")
>>        (match_operand:MODEF 1 "memory_operand" ""))
>>   (set (match_operand:SSEMODEI24 2 "register_operand" "")
>>        (fix:SSEMODEI24 (match_dup 0)))]
>>  "TARGET_SHORTEN_X87_SSE
>>   && peep2_reg_dead_p (2, operands[0])"
>>  [(set (match_dup 2) (fix:SSEMODEI24 (match_dup 1)))]
>>  "")
> 
> I suppose adding && !TARGET_AVOID_VECTOR_DECODE
> would fix it.  Testing that.

I just added "0 &&" to one of the patterns, it also fixed the Fortran
testcase.  Also running a bootstrap...

Do you want to take it from here with your version?


Bernd
Richard Guenther - June 30, 2010, 10:52 a.m.
On Wed, Jun 30, 2010 at 12:41 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 06/30/2010 12:31 PM, Richard Guenther wrote:
>> On Wed, Jun 30, 2010 at 12:19 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>>> On 06/30/2010 12:00 PM, Richard Guenther wrote:
>>>
>>>> We're peepholing in circles after your patch.  The peephole2
>>>> dump isn't very informative - how can I see which peephole2s
>>>> matched?
>>>
>>> Not easily.  Maybe breakpoints on all the gen_peephole2_ functions; we
>>> should probably add that to the dumps.
>>>
>>> Seems to be something involving FIX insns, I suspect
>>>
>>> ;; Shorten x87->SSE reload sequences of fix_trunc?f?i_sse patterns.
>>> (define_peephole2
>>>  [(set (match_operand:MODEF 0 "register_operand" "")
>>>        (match_operand:MODEF 1 "memory_operand" ""))
>>>   (set (match_operand:SSEMODEI24 2 "register_operand" "")
>>>        (fix:SSEMODEI24 (match_dup 0)))]
>>>  "TARGET_SHORTEN_X87_SSE
>>>   && peep2_reg_dead_p (2, operands[0])"
>>>  [(set (match_dup 2) (fix:SSEMODEI24 (match_dup 1)))]
>>>  "")
>>
>> I suppose adding && !TARGET_AVOID_VECTOR_DECODE
>> would fix it.  Testing that.
>
> I just added "0 &&" to one of the patterns, it also fixed the Fortran
> testcase.  Also running a bootstrap...
>
> Do you want to take it from here with your version?

Yes.  I'm through bootstrap and into testing right now.

Richard.

>
> Bernd
>
H.J. Lu - June 30, 2010, 3:48 p.m.
On Wed, Jun 30, 2010 at 2:48 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jun 30, 2010 at 10:50 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 06/30/2010 06:56 AM, H.J. Lu wrote:
>>
>>> This seems to work.
>>
>> Please commit then.  Sorry about the breakage, I guess one has to test
>> changes to i386.md on both targets.
>
> I have committed it.  Bootstrap continues until libjava which still
> fails for me running out of virtual memory (16GB ulimit):
>
> virtual memory exhausted: Cannot allocate memory
> make[3]: *** [gnu/javax/imageio/jpeg.lo] Error 1
> make[3]: *** Waiting for unfinished jobs....
> virtual memory exhausted: Cannot allocate memory
> make[3]: *** [javax/swing/text.lo] Error 1
>

I got bootstrap failure when configured with --with-cpu=atom:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44727

Patch

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 7003f52..c450c38 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -17558,7 +17558,7 @@ 
 ;;  leal    (%edx,%eax,4), %eax

 (define_peephole2
-  [(match_scratch:SI 5 "r")
+  [(match_scratch:P 5 "r")
    (parallel [(set (match_operand 0 "register_operand" "")
 		   (ashift (match_operand 1 "register_operand" "")
 			   (match_operand 2 "const_int_operand" "")))
@@ -17587,9 +17587,12 @@ 

   operands[1] = gen_rtx_PLUS (Pmode, base,
   			      gen_rtx_MULT (Pmode, index, GEN_INT (scale)));
-  if (mode != Pmode)
-    operands[1] = gen_rtx_SUBREG (mode, operands[1], 0);
   operands[5] = base;
+  if (mode != Pmode)
+    {
+      operands[1] = gen_rtx_SUBREG (mode, operands[1], 0);
+      operands[5] = gen_rtx_SUBREG (mode, operands[5], 0);
+    }
   operands[0] = dest;
 })