632221 - jscpucfg should use $(HOST_LDFLAGS) when linking

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: Use $(HOST_LDFLAGS) when linking

To use LTO with clang it is necessary to pass -use-gold-plugin to the driver when linking.

Comment 1

[Andrew Paprocki]

Attachment: Add HOST_LDFLAGS to jscpucfg and HOST_SIMPLE_PROGRAMS

Added HOST_LDFLAGS to HOST_SIMPLE_PROGRAMS too because they are also binaries currently built without them. In addition to what the original requester wants, this breaks non-std gcc installs which need to pass -Wl,-R/path/to/gcc/libs. Without that, jscpucfg and HOST_SIMPLE_PROGRAMS compile, but fail to run with a runtime linker error.

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 530547 [details] [diff] [review]
Add HOST_LDFLAGS to jscpucfg and HOST_SIMPLE_PROGRAMS

Review of attachment 530547 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine with one caveat.

::: js/src/config/rules.mk
@@ +1054,4 @@
>  	$(HOST_LD) -NOLOGO -OUT:$@ -PDB:$(HOST_PDBFILE) $< $(WIN32_EXE_LDFLAGS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
>  else
>  ifneq (,$(HOST_CPPSRCS)$(USE_HOST_CXX))
> +	$(HOST_CXX) $(HOST_OUTOPTION)$@ $(HOST_CXXFLAGS) $(HOST_LDFLAGS) $(INCLUDES) $< $(HOST_LIBS) $(HOST_EXTRA_LIBS)

If you're changing js/src/config/rules.mk, you need to make the same changes to the top-level config/rules.mk as well. (You can just cp js/src/config/rules.mk config/rules.mk and refresh your patch.)

Comment 3

[Andrew Paprocki]

Attachment: Add HOST_LDFLAGS to jscpucfg and HOST_SIMPLE_PROGRAMS

Updated patch to add config/rules.mk per comment above.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 541822 [details] [diff] [review]
Add HOST_LDFLAGS to jscpucfg and HOST_SIMPLE_PROGRAMS

Review of attachment 541822 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/Makefile.in
@@ +909,4 @@
>  endif
>  else
>  jscpucfg$(HOST_BIN_SUFFIX): jscpucfg.cpp Makefile.in
> +	$(HOST_CXX) $(HOST_CXXFLAGS) $(HOST_LDFLAGS) $(JSCPUCFG_DEFINES) $(DEFINES) $(NSPR_CFLAGS) $(HOST_OUTOPTION)$@ $<

I feel like these would be simpler if we didn't need these rules. Can we just make jscpucfg a normal HOST_PROGRAM and add JSCPUCFG_DEFINES to HOST_CXXFLAGS?

Comment 5

[Andrew Paprocki]

Attachment: Add HOST_LDFLAGS to host programs, convert jscpucfg to be a host program

I took your advice and reworked jscpucfg to be a host program like the kw/oplen ones. This adds the HOST_LDFLAGS to the simple programs and cleans up Makefile.in.

Comment 6

[Andrew Paprocki]

Nevermind the latest patch.. it seems jscpucfg has some other magic in there which is why it was left out of HOST_SIMPLE_PROGRAMS by default. The previous patch stands, then to just get this fixed.

Comment 7

[Andrew Paprocki]

Attachment: Test patch for HOST_LD_FLAGS change as well as putting jscpucfg in HOST_SIMPLE_PROGRAMS

Ted, I believe I got the kinks in the previous attempt worked out and I incorporated the custom compilation flags being passed in jscpucfg's target. Please review and if this looks good then this can obsolete the other patch and gives you what you want.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 544198 [details] [diff] [review]
Test patch for HOST_LD_FLAGS change as well as putting jscpucfg in HOST_SIMPLE_PROGRAMS

Review of attachment 544198 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/Makefile.in
@@ +942,5 @@
> +export:: jsautocfg.h
> +
> +# host_jscpucfg is a strange target
> +# Needs to be built with the host compiler but needs to include
> +# the mdcpucfg for the target so it needs the appropriate target defines

Is there a bug on file on killing jscpucfg? If not, can you file one?

Comment 9

[Daniel Holbert [:dholbert]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/51f17131556b

Comment 10

[(no longer active)]

This broke the build <http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Inbound/1310586122.1310587058.31187.gz> so I backed it out:

http://hg.mozilla.org/integration/mozilla-inbound/rev/39738af39421

Comment 11

[Daniel Holbert [:dholbert]]

(Apparently this builds successfully on Linux & Android, but not on Mac & Windows.)

Comment 12

[Andrew Paprocki]

Withdrawing this, as the jscpucfg binary has been removed in mozilla-inbound.