1585351 - Investigate Bug 1460357 and see if it can be backed out

Status: RESOLVED FIXED

(Firefox Build System :: General: Unsupported Platforms, task)

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1585351 - Bug 1460357 disabled AVX instructions for the mingw build; this is no longer needed r?lsalzman

Bug 1460357 disabled AVX because gcc was generating unaligned instructions. But clang doesn't seem
to do that.

Comment 2

[Tom Ritter [:tjr]]

Lee, is there any chance you could point me to a test or test page that definitely uses this code so I can double confirm there's no crash?

Comment 3

[Lee Salzman [:lsalzman]]

People might still build with GCC, so are you sure you can assume we're building with clang?

Comment 4

[Tom Ritter [:tjr]]

(In reply to Lee Salzman [:lsalzman] from comment #3)

People might still build with GCC, so are you sure you can assume we're building with clang?

Sorry - this is specifically about gcc on Windows which we no longer support.

Comment 5

[Lee Salzman [:lsalzman]]

Well, if the MINGW32 check doesn't actually ensnare clang, there is currently no reason to remove this. It's already rolled into a much bigger patch I'm maintaining, so it would be more effort to remove than to just leave it in place.

Comment 6

[Tom Ritter [:tjr]]

(In reply to Lee Salzman [:lsalzman] from comment #5)

Well, if the MINGW32 check doesn't actually ensnare clang, there is currently no reason to remove this. It's already rolled into a much bigger patch I'm maintaining, so it would be more effort to remove than to just leave it in place.

Sorry - yes; the MINGW32 macro matches both mingw-gcc and mingw-clang builds so it is affecting the current mingw builds.

That said, this is - as far as I know - just a performance improvement so it would be fine to wait on this until the next time you rebase your patch and roll it up in that change.

Comment 7

[Lee Salzman [:lsalzman]]

(In reply to Tom Ritter [:tjr] from comment #6)

(In reply to Lee Salzman [:lsalzman] from comment #5)

Well, if the MINGW32 check doesn't actually ensnare clang, there is currently no reason to remove this. It's already rolled into a much bigger patch I'm maintaining, so it would be more effort to remove than to just leave it in place.

Sorry - yes; the MINGW32 macro matches both mingw-gcc and mingw-clang builds so it is affecting the current mingw builds.

That said, this is - as far as I know - just a performance improvement so it would be fine to wait on this until the next time you rebase your patch and roll it up in that change.

If that's the case just go ahead and land this and I'll roll it in.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b50de96607f4
Bug 1460357 disabled AVX instructions for the mingw build; this is no longer needed r=lsalzman

Comment 9

[Cristian Brindusan [:cbrindusan]]

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