jscpucfg should use $(HOST_LDFLAGS) when linking

RESOLVED INVALID

Status

()

Core
JavaScript Engine
RESOLVED INVALID
7 years ago
7 years ago

People

(Reporter: espindola, Assigned: Andrew Paprocki)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Created attachment 510423 [details] [diff] [review]
Use $(HOST_LDFLAGS) when linking

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

Comment 1

7 years ago
Created attachment 530547 [details] [diff] [review]
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.
Attachment #510423 - Attachment is obsolete: true
Attachment #530547 - Flags: review?(ted.mielczarek)
Assignee: general → andrew
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.)
Attachment #530547 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 3

7 years ago
Created attachment 541822 [details] [diff] [review]
Add HOST_LDFLAGS to jscpucfg and HOST_SIMPLE_PROGRAMS

Updated patch to add config/rules.mk per comment above.
Attachment #541822 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

7 years ago
Attachment #530547 - Attachment is obsolete: true
Attachment #541822 - Flags: review?(ted.mielczarek) → review+
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?
(Assignee)

Comment 5

7 years ago
Created attachment 544142 [details] [diff] [review]
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.
Attachment #541822 - Attachment is obsolete: true
Attachment #544142 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

7 years ago
Attachment #541822 - Attachment is obsolete: false
(Assignee)

Updated

7 years ago
Attachment #544142 - Attachment is obsolete: true
Attachment #544142 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 6

7 years ago
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.
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Assignee)

Comment 7

7 years ago
Created attachment 544198 [details] [diff] [review]
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.
Attachment #544198 - Flags: review?(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?
Attachment #544198 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Updated

7 years ago
Attachment #541822 - Attachment is obsolete: true
http://hg.mozilla.org/integration/mozilla-inbound/rev/51f17131556b
Keywords: checkin-needed
Whiteboard: [inbound]
Version: unspecified → Trunk
(Apparently this builds successfully on Linux & Android, but not on Mac & Windows.)
(Assignee)

Comment 12

7 years ago
Withdrawing this, as the jscpucfg binary has been removed in mozilla-inbound.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.