diff mbox

PR 53975

Message ID 5017BC99.5020809@ispras.ru
State New
Headers show

Commit Message

Andrey Belevantsev July 31, 2012, 11:08 a.m. UTC
Hello,

This PR is about wrong speculation of an insn that doesn't support storing 
NaT bits done by the selective scheduler (more details in the PR audit 
trail).  The reason for this is the wrong one-liner patch committed last 
year, the fix is to revert that patch and to clarify the comment before the 
patched code.

Bootstrapped and tested on ia64, approved by Alexander offline.  No test as 
I don't know how to check whether an insn got moved through another insn.

Andrey

         PR target/53975

         * sel-sched-ir.c (has_dependence_note_reg_use): Clarify comment.

         Revert
         2011-08-04  Sergey Grechanik  <mouseentity@ispras.ru>

         * sel-sched-ir.c (has_dependence_note_reg_use): Call ds_full_merge
         only if producer writes to the register given by regno.

Comments

Andrey Belevantsev Oct. 16, 2012, 7:46 a.m. UTC | #1
Hello,

On 31.07.2012 15:08, Andrey Belevantsev wrote:
> Hello,
>
> This PR is about wrong speculation of an insn that doesn't support storing
> NaT bits done by the selective scheduler (more details in the PR audit
> trail).  The reason for this is the wrong one-liner patch committed last
> year, the fix is to revert that patch and to clarify the comment before the
> patched code.
>
> Bootstrapped and tested on ia64, approved by Alexander offline.  No test as
> I don't know how to check whether an insn got moved through another insn.

Finally I've got to porting this to 4.7.  The patch was successfully tested 
on ia64, I'll commit this after double checking on x86-64.  The patch 
should be committed with the fix for PR 53701, which I will do right after 
this one.

Andrey

2012-10-16  Andrey Belevantsev  <abel@ispras.ru>

	Backport from mainline
	2012-07-31  Andrey Belevantsev  <abel@ispras.ru>
         PR target/53975

         * sel-sched-ir.c (has_dependence_note_reg_use): Clarify comment.
         Revert
         2011-08-04  Sergey Grechanik  <mouseentity@ispras.ru>
         * sel-sched-ir.c (has_dependence_note_reg_use): Call ds_full_merge
         only if producer writes to the register given by regno.
	
Index: gcc/sel-sched-ir.c
===================================================================
*** gcc/sel-sched-ir.c  (revision 192488)
--- gcc/sel-sched-ir.c  (working copy)
*************** has_dependence_note_reg_use (int regno)
*** 3224,3230 ****
         if (reg_last->clobbers)
         *dsp = (*dsp & ~SPECULATIVE) | DEP_ANTI;

!       /* Handle BE_IN_SPEC.  */
         if (reg_last->uses)
         {
           ds_t pro_spec_checked_ds;
--- 3224,3234 ----
         if (reg_last->clobbers)
         *dsp = (*dsp & ~SPECULATIVE) | DEP_ANTI;

!       /* Merge BE_IN_SPEC bits into *DSP when the dependency producer
!        is actually a check insn.  We need to do this for any register
!        read-read dependency with the check unless we track properly
!        all registers written by BE_IN_SPEC-speculated insns, as
!        we don't have explicit dependence lists.  See PR 53975.  */
         if (reg_last->uses)
         {
           ds_t pro_spec_checked_ds;
*************** has_dependence_note_reg_use (int regno)
*** 3232,3240 ****
           pro_spec_checked_ds = INSN_SPEC_CHECKED_DS 
(has_dependence_data.pro);
           pro_spec_checked_ds = ds_get_max_dep_weak (pro_spec_checked_ds);

!         if (pro_spec_checked_ds != 0
!             && bitmap_bit_p (INSN_REG_SETS (has_dependence_data.pro), regno))
!           /* Merge BE_IN_SPEC bits into *DSP.  */
             *dsp = ds_full_merge (*dsp, pro_spec_checked_ds,
                                   NULL_RTX, NULL_RTX);
         }
--- 3236,3242 ----
           pro_spec_checked_ds = INSN_SPEC_CHECKED_DS 
(has_dependence_data.pro);
           pro_spec_checked_ds = ds_get_max_dep_weak (pro_spec_checked_ds);

!         if (pro_spec_checked_ds != 0)
             *dsp = ds_full_merge (*dsp, pro_spec_checked_ds,
                                   NULL_RTX, NULL_RTX);
         }
diff mbox

Patch

Index: gcc/sel-sched-ir.c
===================================================================
*** gcc/sel-sched-ir.c	(revision 190004)
--- gcc/sel-sched-ir.c	(revision 190005)
*************** has_dependence_note_reg_use (int regno)
*** 3228,3234 ****
        if (reg_last->clobbers)
  	*dsp = (*dsp & ~SPECULATIVE) | DEP_ANTI;
  
!       /* Handle BE_IN_SPEC.  */
        if (reg_last->uses)
  	{
  	  ds_t pro_spec_checked_ds;
--- 3228,3238 ----
        if (reg_last->clobbers)
  	*dsp = (*dsp & ~SPECULATIVE) | DEP_ANTI;
  
!       /* Merge BE_IN_SPEC bits into *DSP when the dependency producer
! 	 is actually a check insn.  We need to do this for any register
! 	 read-read dependency with the check unless we track properly
! 	 all registers written by BE_IN_SPEC-speculated insns, as
! 	 we don't have explicit dependence lists.  See PR 53975.  */
        if (reg_last->uses)
  	{
  	  ds_t pro_spec_checked_ds;
*************** has_dependence_note_reg_use (int regno)
*** 3236,3244 ****
  	  pro_spec_checked_ds = INSN_SPEC_CHECKED_DS (has_dependence_data.pro);
  	  pro_spec_checked_ds = ds_get_max_dep_weak (pro_spec_checked_ds);
  
! 	  if (pro_spec_checked_ds != 0
! 	      && bitmap_bit_p (INSN_REG_SETS (has_dependence_data.pro), regno))
! 	    /* Merge BE_IN_SPEC bits into *DSP.  */
  	    *dsp = ds_full_merge (*dsp, pro_spec_checked_ds,
  				  NULL_RTX, NULL_RTX);
  	}
--- 3240,3246 ----
  	  pro_spec_checked_ds = INSN_SPEC_CHECKED_DS (has_dependence_data.pro);
  	  pro_spec_checked_ds = ds_get_max_dep_weak (pro_spec_checked_ds);
  
! 	  if (pro_spec_checked_ds != 0)
  	    *dsp = ds_full_merge (*dsp, pro_spec_checked_ds,
  				  NULL_RTX, NULL_RTX);
  	}