Page MenuHomePhabricator

[zorg] Add Clang ppc64le cross-targeting buildbot on AIX
ClosedPublic

Authored by Xiangling_L on Jan 27 2021, 7:35 AM.

Diff Detail

Event Timeline

Xiangling_L created this revision.Jan 27 2021, 7:35 AM

@gkistanova Hi Galina, we are trying to set up a cross-targeting buildbot for AIX platform. And I sent you an email on Wed, Jan 20th, 2021 requesting access for the new build bot.

Can you please confirm that you received it. Thanks.

buildbot/osuosl/master/config/builders.py
634

I think having the host platform described more clearly makes the non-native default target more obvious.

655

There's at least one typo to correct. Just use the value from a build for ppc64le Linux.

Xiangling_L marked 2 inline comments as done.

Addressed the comments;

buildbot/osuosl/master/config/builders.py
637

Please consider changing this to match the name.

651–652

I think you reported "offline" that you needed to add -qaltivec.

Xiangling_L marked 2 inline comments as done.Feb 2 2021, 7:40 AM
Xiangling_L added inline comments.
buildbot/osuosl/master/config/builders.py
637

Oops... I missed this.

651–652

Thanks for reminding. I will update this. I was waiting for build to finish. It usually took around 6 hours.

Xiangling_L marked 2 inline comments as done.

Update build dir name accordingly;
Ad -qaltivec into cmake config;

@Xiangling_L, sorry if this is adding extra respin test cycles. I've done as much review this time around as I can do for the patch as it is currently posted. Thanks.

buildbot/osuosl/master/config/builders.py
635

Minor comment: Usage of the "ppc" tag seems to imply it is a generic tag for anything Power-related. It makes sense to add it here.

643

Not sure if other people have a different opinion about this, but I think being explicit for some settings used by "nearby" entries make sense.

646

Most configurations using involving Ninja appear to use getCmakeWithNinjaBuildFactory. The ones that don't mostly also have make='ninja'.

647

My understanding is that "llvm" is not needed in the LLVM_ENABLE_PROJECTS list. I guess we could keep it, but I don't think we should keep the trailing semicolon.

daltenty added inline comments.Feb 2 2021, 9:03 AM
buildbot/osuosl/master/config/builders.py
646

Generator options already seem to exist in the ClangBuilder implementation, so I don't think we should be passing them here.

656

The binutils directory here is used for the gold plugin, so we don't need it for AIX.

657

Looks like the jobs property on the worker will set the lit threads, so we don't need that part I think.

buildbot/osuosl/master/config/workers.py
121

Just noting we'll have to boost this when we add the native builder for example (we're still waiting on more machine resources though).

Xiangling_L marked 8 inline comments as done.

Addressed the comments;

LGTM with some last changes based on @daltenty's comments.

buildbot/osuosl/master/config/builders.py
641

The make='ninja' line is not needed given @daltenty's comment.

646

@daltenty, just confirming you are saying that https://reviews.llvm.org/source/zorg/browse/main/zorg/buildbot/builders/ClangBuilder.py gives us Ninja by default?

This revision is now accepted and ready to land.Feb 2 2021, 1:33 PM
daltenty added inline comments.Feb 2 2021, 3:26 PM
buildbot/osuosl/master/config/builders.py
646

@daltenty, just confirming you are saying that https://reviews.llvm.org/source/zorg/browse/main/zorg/buildbot/builders/ClangBuilder.py gives us Ninja by default?

Certainly seems that way, every generator option is hard coded to Ninja

Xiangling_L marked 3 inline comments as done.

Remove make='ninja';

gkistanova requested changes to this revision.Feb 4 2021, 8:06 PM

Almost there. One more change it will be ready to land.

buildbot/osuosl/master/config/builders.py
649

No need to specify the LLVM_ENABLE_PROJECTS. The build factory will take care of this for you.

This revision now requires changes to proceed.Feb 4 2021, 8:06 PM
Xiangling_L marked an inline comment as done.Feb 8 2021, 7:11 AM
Xiangling_L added inline comments.
buildbot/osuosl/master/config/builders.py
649

Thanks, I will update the patch accordingly.

Xiangling_L marked an inline comment as done.

Removed redundant -DLLVM_ENABLE_PROJECTS option;

gkistanova accepted this revision.Feb 8 2021, 9:18 PM

Thanks!

This revision is now accepted and ready to land.Feb 8 2021, 9:18 PM