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: PR bootstrap/18532: libgcc.mk isn't parallel build safe for multilib


On Thu, Dec 02, 2004 at 11:14:58AM -0800, Zack Weinberg wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> 
> > On Thu, Dec 02, 2004 at 10:44:59AM -0800, Zack Weinberg wrote:
> >> "H. J. Lu" <hjl@lucon.org> writes:
> >> 
> >> >> I don't see the need for the dependency on $prev_extra, could you
> >> >> explain why that is there?  I'd think it would be preferable either
> >> >> to invoke recursive make just once with *all* the EXTRA_MULTILIB_PARTS
> >> >> targets (across all the multilibs) or else to allow the recursive
> >> >> makes for each multilib to run in parallel.
> >> >
> >> > We need it since each multilib is built with
> >> >
> >> > $(MAKE) T=$dir ...
> >> >
> >> > "$dir" is different in each build.
> >> 
> >> I see.
> >> 
> >> > We may not do it in parallel safely, which is PR bootstrap/18532. We
> >> > may only do it safely one multilib at a time.
> >> 
> >> Can you actually demonstrate a problem if the $prev_extra dependency
> >> is removed, or is this just a theoretical issue?
> >> 
> >
> > It isn't a problem for x86-64 since it only has 2 sets and the default
> > one is built before libgcc.mk is reached. It may be a problem for
> > targets with more than 2 sets since you may have
> >
> > make T=dir1/ ...
> > make T=dir2/ ...
> >
> > running at the same time. It is the same as running
> >
> > make T=dir1/ ...
> > make T=dir1/ ...
> 
> Demonstrate the problem means show me a failed make transcript.  I'm
> not going to believe that there's a problem until I see it actually
> fail in practice.  [I strongly suspect that the sort of conflict over
> temporary file names that prompted the original PR will never happen
> in practice for disjoint multilibs, since the temp files will be in
> different directories.  There might be other problems, of course.
> I'll believe it when I see a failure.]
> 
> >> Now it's not always set.  You have to hoist the existing definition
> >> out of the SHLIB_LINK logic, like I said.
> >> 
> >
> > Here is the new patch.
> 
> One more nitpick:
> 
> > +    # We can't have individual element in EXTRA_MULTILIB_PARTS as a
> > +    # target since it won't be parallel build safe.  We bundle them
> > +    # together and pass them to Makefile. libgcc.mk has no knowledge
> > +    # of if and how EXTRA_MULTILIB_PARTS are built.
> 
> This comment does not communicate the problem very well.  Suggest
> instead
> 
> # Each of the EXTRA_MULTILIB_PARTS is built by recursive invocation of
> # the parent Makefile.  We must do this just once for each multilib,
> # passing it all the EXTRA_MULTILIB_PARTS as simultaneous goal
> # targets, so that rules which cannot execute simultaneously are
> # properly serialized.
> 
> Also your ChangeLog is not particularly informative.
> 

Here are the updated patch and the libgcc.mk diff.

H.J.
----
2004-12-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR bootstrap/18532
	* mklibgcc.in: Build one set of EXTRA_MULTILIB_PARTS for
	multilib at a time. Don't build the default set. Don't add
	EXTRA_MULTILIB_PARTS to shared libunwind nor libgcc. Remove
	filter for shared libunwind and libgcc.

--- gcc/mklibgcc.in.multilib	2004-12-01 10:11:00.000000000 -0800
+++ gcc/mklibgcc.in	2004-12-02 12:04:32.725925542 -0800
@@ -160,6 +160,11 @@ for ml in $MULTILIBS; do
   libgcc_s_so=
   libunwind_a=
   libunwind_so=
+  if [ "$dir" = . ]; then
+    suffix=
+  else
+    suffix=`echo $dir | sed s,/,_,g`
+  fi
 
   if [ "$LIBUNWIND" ]; then
     libunwind_a=$dir/libunwind.a
@@ -177,7 +182,6 @@ for ml in $MULTILIBS; do
 	  libunwind_soname=libunwind
 	fi
       else
-        suffix=`echo $dir | sed s,/,_,g`
 	libgcc_eh_a=$dir/libgcc_eh.a
 	libgcc_s_so_base=$dir/libgcc_s_${suffix}
 	libgcc_s_so=${libgcc_s_so_base}${SHLIB_EXT}
@@ -655,37 +659,36 @@ for ml in $MULTILIBS; do
   done
 
   # EXTRA_MULTILIB_PARTS.
-  echo
-  for f in $EXTRA_MULTILIB_PARTS; do
-    case $dir in
-    .) out=$f ; t= ;;
-    *) out=$dir/$f ; t=$dir/ ;;
-    esac
-    case $out in
-    # Prevent `make' from interpreting $out as a macro assignment
-    *'$(EQ)'*) targ="T_TARGET=$out T_TARGET" ;;
-    *) targ=$out ;;
-    esac
-
-    echo $out: stmp-dirs
-    echo "	$make_compile" \\
-    echo '	  LIBGCC2_CFLAGS="$(LIBGCC2_CFLAGS)' $flags '" ' \\
-    echo '	  MULTILIB_CFLAGS="'$flags'"' T=$t $targ
-    echo "all: $out"
-
-    # Make libunwind.so and libgcc_s.so depend on these, since they are
-    # likely to be implicitly used by the link process.  However, we must
-    # then arrange to remove them from the explicit object list generated
-    # from $^ - see below.
-    if [ "$libgcc_s_so" ]; then
-      libgcc_s_so_extra="$libgcc_s_so_extra $out"
-      echo "$libgcc_s_so: $out"
-    fi
-    if [ "$libunwind_so" ]; then
-      libunwind_so_extra="$libunwind_so_extra $out"
-      echo "$libunwind_so: $out"
+  if [ -n "$EXTRA_MULTILIB_PARTS" ]; then
+    # Each of the EXTRA_MULTILIB_PARTS is built by recursive invocation
+    # of the parent Makefile.  We must do this just once for each
+    # multilib, passing it all the EXTRA_MULTILIB_PARTS as
+    # simultaneous goal targets, so that rules which cannot execute
+    # simultaneously are properly serialized.
+    
+    # We don't need to build the default ones since they have been
+    # built before we get here.
+    if [ $dir != "." ]; then
+      extra=
+      t=$dir/
+      echo
+      for f in $EXTRA_MULTILIB_PARTS; do
+	out=$dir/$f
+	case $out in
+	# Prevent `make' from interpreting $out as a macro assignment
+	*'$(EQ)'*) targ="T_TARGET=$out T_TARGET" ;;
+	*) targ=$out ;;
+	esac
+	extra="$extra $targ"
+      done
+
+      echo extra$suffix: stmp-dirs
+      echo "	$make_compile" \\
+      echo '	  LIBGCC2_CFLAGS="$(LIBGCC2_CFLAGS)' $flags '" ' \\
+      echo '	  MULTILIB_CFLAGS="'$flags'"' T=$t $extra
+      echo "all: extra$suffix"
     fi
-  done
+  fi
 
   # Library build rules.
   dir=`echo ${ml} | sed -e 's/;.*$//' -e 's/=/$(EQ)/g'`
@@ -760,7 +763,7 @@ EOF
     echo "	$SHLIB_LINK" \
 	 | sed -e "s%@multilib_flags@%$flags%g" \
 	       -e "s%@multilib_dir@%$dir%g" \
-	       -e "s%@shlib_objs@%\$(filter-out $libgcc_s_so_extra,\$(objects))%g" \
+	       -e "s%@shlib_objs@%\$(objects)%g" \
 	       -e "s%@shlib_base_name@%$libgcc_s_so_base%g" \
 	       -e "s%@shlib_so_name@%$libgcc_s_soname%g" \
 	       -e "s%@shlib_map_file@%$mapfile%g" \
@@ -775,7 +778,7 @@ EOF
     echo "	$SHLIBUNWIND_LINK" \
 	   | sed -e "s%@multilib_flags@%$flags%g" \
 		 -e "s%@multilib_dir@%$dir%g" \
-		 -e "s%@shlib_objs@%\$(filter-out $libunwind_so_extra,\$(objects))%g" \
+		 -e "s%@shlib_objs@%\$(objects)%g" \
 		 -e "s%@shlib_base_name@%$libunwind_so_base%g" \
 		 -e "s%@shlib_so_name@%$libunwind_soname%g" \
 		 -e "s%@shlib_dir@%$shlib_dir%g" \

Attachment: y
Description: Text document


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