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] Fix find_inc in the scheduler (PR target/62025)


On Thu, Aug 14, 2014 at 11:49:37AM +0200, Jakub Jelinek wrote:
> On Thu, Aug 14, 2014 at 11:34:04AM +0200, Jakub Jelinek wrote:
> > So, to set DEP_MULTIPLE even in the case where ask_depencency_caches
> > returns DEP_PRESENT, you'd need to find the old dependency anyway (isn't
> > that going to be expensive and totally kill all the effects of
> > true_dependency_cache?) and set DEP_MULTIPLE on it if not already set.
> 
> Something like (untested except for the openssl s390 testcase), haven't
> checked what effects it has on the number of successful find_inc cases.

Here is the same thing with ChangeLog entry, after bootstrap/regtest
on x86_64-linux and i686-linux.

With extra instrumentation (pretty much not removing the find_inc hunk and
adding the patch from yesterday too + statistics accumulation) shows
comparable amount of successful parse_add_or_inc in find_inc (around 1.2M), but to my
surprise still about 13000 cases where DEP_MULTIPLE is not set, yet there
are clearly more than one dependency between mem_inc and inc_insn.

The only testcase I've looked so far has been
-m32 -O3 -fomit-frame-pointer -funroll-loops -funroll-all-loops 990517-1.c
and what I see there is we have
sp += 24, clobber flags;
...
sp += 16, clobber flags;
...
flags = ([sp+1096] & eax) != 0;
The dep in between the middle insn and last insn has DEP_MULTIPLE set,
one dep there has been for sp setter vs. sp user and the other for
clobber flags vs. flags setter.  But for the first insn and third insn pair,
only the clobber flags vs. flags setter dependency has been recorded,
presumably the one on sp not, because there is some other setter after that.
I hope the scheduler doesn't attempt to swap sp += 24 with flags setter
because of the sp += 16 vs. flags setter dependency and sp += 24 vs. sp +=
16 dependency, but I feel kind of uneasy with find_inc assuming the recorded
dependency is the one for the mem_reg0, when in this case the dependency is
there for completely different register.

2014-08-14  Jakub Jelinek  <jakub@redhat.com>

	PR target/62025
	* sched-deps.c (add_or_update_dep_1): If ask_dependency_caches
	returned DEP_PRESENT, make sure to set DEP_MULTIPLE on present_dep.
	(find_inc): Revert 2014-08-13 change.

--- gcc/sched-deps.c.jj	2014-08-12 17:06:26.000000000 +0200
+++ gcc/sched-deps.c	2014-08-14 11:46:25.509631874 +0200
@@ -1233,6 +1233,13 @@ add_or_update_dep_1 (dep_t new_dep, bool
       switch (ask_dependency_caches (new_dep))
 	{
 	case DEP_PRESENT:
+	  dep_t present_dep;
+	  sd_iterator_def sd_it;
+      
+	  present_dep = sd_find_dep_between_no_cache (DEP_PRO (new_dep),
+						      DEP_CON (new_dep),
+						      resolved_p, &sd_it);
+	  DEP_MULTIPLE (present_dep) = 1;
 	  return DEP_PRESENT;
 
 	case DEP_CHANGED:
@@ -4752,23 +4759,6 @@ find_inc (struct mem_inc_info *mii, bool
 		goto next;
 	      }
 
-	  /* The inc instruction could have clobbers, make sure those
-	     registers are not used in mem insn.  */
-	  FOR_EACH_INSN_DEF (def, mii->inc_insn)
-	    if (!reg_overlap_mentioned_p (DF_REF_REG (def), mii->mem_reg0))
-	      {
-		df_ref use;
-		FOR_EACH_INSN_USE (use, mii->mem_insn)
-		  if (reg_overlap_mentioned_p (DF_REF_REG (def),
-					       DF_REF_REG (use)))
-		    {
-		      if (sched_verbose >= 5)
-			fprintf (sched_dump,
-				 "inc clobber used in store failure.\n");
-		      goto next;
-		    }
-	      }
-
 	  newaddr = mii->inc_input;
 	  if (mii->mem_index != NULL_RTX)
 	    newaddr = gen_rtx_PLUS (GET_MODE (newaddr), newaddr,


	Jakub


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