Page MenuHomeWildfire Games
Differential D1619

Enable support for powerpc64 systems
ClosedPublic
Actions

Authored by tpearson-raptor on Aug 21 2018, 12:03 AM.

Details

Reviewers
Stan
Commits
rP24995: Enable support for powerpc64 systems. New problems with NVTT will be addressed…
Summary

All features tested and work in actual gameplay.

Test Plan

All features tested and work in gameplay tests. Internal checks pass with no errors.

Test hardware:
Talos II system with dual POWER9 CPUs, AMD Radeon WX7100 graphics, open amdgpu driver stack.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

tpearson-raptor created this revision.Aug 21 2018, 12:03 AM

Requires http://trac.wildfiregames.com/ticket/5292 . This is why libraries/source/spidermonkey/mozjs-38.2.1.rc0.tar.bz2 has been flagged as needing changes.

tpearson-raptor added reviewers: elexis, Itms, s0600204.Aug 21 2018, 12:08 AM

Screenshot from working build: https://imgur.com/a/Jvclj0j

tpearson-raptor updated this revision to Diff 6871.Aug 21 2018, 12:11 AM

Add omitted copyright line

lyv added a subscriber: lyv.Aug 21 2018, 3:11 AM

IIRC, I have seen diff files in the repo for changing the file. So I suppose you can just add the diff to this revision.
I’m not sure though. Or you could just wait for a reviewers instructions.

s0600204 requested changes to this revision.Aug 21 2018, 6:44 PM
s0600204 added a subscriber: wraitii.
In #5292, @tpearson-raptor asked:

Is there a process to patch the stock .tar.bz2 sources?

Yes, yes there is.

When spidermonkey is to be compiled, the .tar.bz2 is uncompressed, then patch.sh (in the same directory as the .tar.bz2 archive) is run. This applies the *.diff files (also in the same directory) to the newly-uncompressed source.

So. Instead of replacing the .tar.bz2 file, could you create a .diff file, and set it up to be used inside patch.sh (with a brief/succinct/descriptive comment as to what it does/is for).

As to nvtt, we do patch the source (as you have done below), but the specific changeset applied should also be contained inside a .patch file inside libraries/source/nvtt/patches and a line should be added to libraries/source/nvtt/README.txt describing why its needed.

Thanks for your efforts so far!

There is a plan to upgrade SpiderMonkey to version 45.0.2 (D1510), as a waypoint on the route to eventually upgrade to version 60 or 62 (no changeset supplied yet afaik). @wraitii was working on that, so might have some comments to make here. For extra bonus points, you could look into checking that SM45 works on ppc64 systems as well. (Any changes required to make SM45 work on ppc64 that are not needed to make SM38 work on ppc64 should be in its own separate, albeit dependant, revision.)

A note about copyright notices: We do not mention individual persons or companies. If someone wishes to see who last changed a file, or specific lines within a file, then that can be obtained by querying the version-control systems. (We're usually good with attributions in commit messages.)

This appears to be your first contribution. Welcome to the project!

As such, you may include a small diff to add yourself to the credits: ./binaries/data/mods/public/gui/credits/texts/programming.json (Not compulsory, a small number of people prefer to go uncredited.)

This revision now requires changes to proceed.Aug 21 2018, 6:44 PM
tpearson-raptor updated this revision to Diff 6873.Aug 21 2018, 7:24 PM

Move spidermonkey and nvtt patches into dedicated patch files
Fix copyright lines per instructions

tpearson-raptor added a comment.Aug 21 2018, 7:26 PM

Thanks for the detailed instructions! Patchset has been updated.

To be best of my knowledge spidermonkey just works on ppc64 in later versions -- the problem here was the old 3.8 version. There are a number of folks maintaining Firefox now on ppc64, so support going forward looks fine.

elexis removed a reviewer: elexis.Aug 21 2018, 7:58 PM

SM52, not 60/62 afaics unless we want to go with unreleased SM.

s0600204 added a comment.Aug 24 2018, 6:52 PM

Better, thank you.

I don't have a ppc64 system, so I'll have to take your word for it that this works, but I can confirm that these changes don't mess up compilation on my x86_64 system.

I'm also not that familiar with c++, so I'd like someone else take a look at this before its accepted.

libraries/source/spidermonkey/patch.sh
57 ↗(On Diff #6873)

-p5

Stan added a subscriber: Stan.Jan 9 2019, 6:09 PM

Will probably need a rebase. Also copyright years need to be bumped.Thanks for the work so far.

Stan added a comment.Sep 18 2019, 7:40 PM

Needs to be fixed because of SM45.

s0600204 requested changes to this revision.Jun 18 2020, 12:22 AM

I'm not sure if you're still around (apologies this has taken so long), but as @Stan points out - in the intervening time between your last update and now some things have changed:

  • We've updated spidermonkey from version 38 to version 45
  • We've updated nvtt from 2.0.8 to 2.1.1
  • We no longer use premake4
This revision now requires changes to proceed.Jun 18 2020, 12:22 AM
asterix added a subscriber: asterix.Dec 21 2020, 4:20 PM

Can we have this in alpha 24 with https://code.wildfiregames.com/D3162, please @Stan @s0600204

Stan added a comment.Dec 21 2020, 6:36 PM

Why?

vladislavbelov added a subscriber: vladislavbelov.Dec 22 2020, 8:49 AM

The patch needs context.

libraries/source/spidermonkey/FixPowerPC64Builds.diff
19–20 ↗(On Diff #6873)

Why so? It's not initialized yet?

source/graphics/TextureConverter.cpp
48–50

It probably already exists.

source/lib/alignment.h
87

Which tests and why?

asterix added a comment.Dec 22 2020, 10:35 AM
In D1619#143837, @Stan wrote:

Why?

because if we include e2k architecture it is similar to also include this one.

Stan added a comment.Dec 22 2020, 11:22 AM

Well it's a bit different. Also we have no one with a ppc64 around.

asterix added a comment.Dec 22 2020, 11:33 AM
In D1619#143908, @Stan wrote:

Well it's a bit different. Also we have no one with a ppc64 around.

Well is not Ricotz able to test it on the package system of Ubuntu?

Stan added a comment.Dec 22 2020, 3:41 PM

No he is not. It's not part of the CIs on Ubuntu. Plus, those are only headless CI, they do not support the visual side of things.

tpearson-raptor added a comment.Dec 22 2020, 8:30 PM
In D1619#143908, @Stan wrote:

Well it's a bit different. Also we have no one with a ppc64 around.

I'm happy to test on my Talos II system (Debian) if the patch set is moving toward merge...

Stan added a comment.Dec 22 2020, 10:21 PM

Well first it needs to be rebased. Can you do that?
Then address the inline comments :)
Also can you add context to your patch ?
For the nvtt changes it's better to upstream them and you don't need to include the changes in the cpp files if you include the diff file

Stan edited reviewers, added: Stan; removed: Itms, s0600204.Mar 3 2021, 11:25 AM
This revision now requires review to proceed.Mar 3 2021, 11:25 AM
Stan updated this revision to Diff 16254.Mar 3 2021, 11:25 AM
Stan edited reviewers, added: Itms, s0600204; removed: Stan.

Rebase. @tpearson-raptor can you test again now?

Harbormaster completed remote builds in B15962: Diff 16254.Mar 3 2021, 11:25 AM
Stan edited reviewers, added: Stan; removed: Itms, s0600204.Mar 3 2021, 11:26 AM
Stan updated this revision to Diff 16255.Mar 3 2021, 11:31 AM

years

Harbormaster completed remote builds in B15963: Diff 16255.Mar 3 2021, 11:31 AM
Stan accepted this revision.Mar 3 2021, 4:11 PM
This revision is now accepted and ready to land.Mar 3 2021, 4:11 PM
Closed by commit rP24995: Enable support for powerpc64 systems. New problems with NVTT will be addressed… (authored by Stan). · Explain WhyMar 3 2021, 4:13 PM
This revision was automatically updated to reflect the committed changes.
Stan added a commit: rP24995: Enable support for powerpc64 systems. New problems with NVTT will be addressed….
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Mar 3 2021, 4:13 PM

Revision Contents
Changeset List

PathSize
build/
premake/
4 lines
4 lines
libraries/
source/
nvtt/
src/
src/
nvcore/
poshlib/
2 lines
nvmath/
4 lines
nvtt/
squish/
9 lines
spidermonkey/
source/
graphics/
4 lines
lib/
11 lines
2 lines
sysdep/
8 lines
arch/
ppc64/
50 lines
ps/
GameSetup/
1 line

Diff 6871

View Options

build/premake/premake4.lua

Loading...
View Options

build/premake/premake5.lua

Loading...
View Options

libraries/source/nvtt/src/src/nvcore/poshlib/posh.h

Loading...
View Options

libraries/source/nvtt/src/src/nvmath/Plane.h

Loading...
View Options

libraries/source/nvtt/src/src/nvtt/squish/simd_ve.h

Loading...
View Options

libraries/source/spidermonkey/mozjs-38.2.1.rc0.tar.bz2

Loading...
View Options

source/graphics/TextureConverter.cpp

Loading...
View Options

source/lib/alignment.h

Loading...
View Options

source/lib/byte_order.h

Loading...
View Options

source/lib/sysdep/arch.h

Loading...
View Options

source/lib/sysdep/arch/ppc64/ppc64.cpp

Loading...
View Options

source/ps/GameSetup/HWDetect.cpp

Loading...
Wildfire Games · Phabricator