961287 - don't lto the libvpx asm_enc_offsets.S files we just assemble for offsets

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Trevor Saunders (:tbsaunde)]

for one thing its pointless to lto them since they are never linked into anything, and its harmful because then the assembly file doesn't contain any useful asm.  So lets just not pass lto flags to them.

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: bug 961287 - don't lto files that are only compiled to assembly to get offsets

this is pretty ugh, but I'm not really sure what better option there is...  I'm also not really sure how those lines can be wrapped, but I guess they can somehow?

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8361986 [details] [diff] [review]
bug 961287 - don't lto files that are only compiled to assembly to get offsets

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

::: media/libvpx/Makefile.in
@@ +52,5 @@
>  
>  # We can extract the asm offsets directly from generated assembly using inline
>  # asm. This is the preferred method.
>  
> +vpx_scale_asm_offsets.s: CFLAGS := -DINLINE_ASM $(filter-out -f%-lto-objects -flto%,$(CFLAGS))

Just use CFLAGS := -DINLINE_ASM. We actually shouldn't need any flags here since all we care in these files are offsets in some data tables.

Comment 3

[Trevor Saunders (:tbsaunde)]

Attachment: bug 961287 - don't lto files that are only compiled to assembly to get offsets

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8377772 [details] [diff] [review]
bug 961287 - don't lto files that are only compiled to assembly to get offsets

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

::: media/libvpx/Makefile.in
@@ +52,5 @@
>  
>  # We can extract the asm offsets directly from generated assembly using inline
>  # asm. This is the preferred method.
>  
> +vpx_scale_asm_offsets.s: CFLAGS := -DINLINE_ASM

Maybe add a comment to make it clear that this *is* meant to replace the entire CFLAGS, to avoid any future regression.

Comment 5

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e43584cd2994

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e43584cd2994