Closed Bug 961287 Opened 7 years ago Closed 6 years ago

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

Categories

(Firefox Build System :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(1 file, 1 obsolete file)

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.
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?
Attachment #8361986 - Flags: review?(mh+mozilla)
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.
Attachment #8361986 - Flags: review?(mh+mozilla) → review-
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.
Attachment #8377772 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/e43584cd2994
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.