This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Extra test case for LDARX
ClosedPublic

Authored by Conanap on Jul 13 2021, 11:54 AM.

Details

Summary

An extra test case added for the builtin __LDARX.

Diff Detail

Event Timeline

Conanap created this revision.Jul 13 2021, 11:54 AM
Conanap requested review of this revision.Jul 13 2021, 11:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2021, 11:54 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Conanap added reviewers: Restricted Project, nemanjai.Jul 13 2021, 11:54 AM
nemanjai requested changes to this revision.Jul 13 2021, 7:26 PM
nemanjai added inline comments.
clang/test/CodeGen/builtins-ppc-xlcompat-check-ldarx-opt.ll
2

Please pre-optimize this test case by running opt -O3 on it (or produce it with clang -O3 -S -emit-llvm).

This revision now requires changes to proceed.Jul 13 2021, 7:26 PM
nemanjai added inline comments.Jul 13 2021, 7:27 PM
clang/test/CodeGen/builtins-ppc-xlcompat-check-ldarx-opt.ll
150

Get rid of all the metadata please.

Sorry for raising an unrelated topic here, but I can't reach @Conanap directly via the mail from the git commits: @Conanap could you please create the git branches for your patches in your own Github fork instead of the main LLVM repo? LLVM's policy is to have working branches in everyone's private fork (even though I don't think we explicitly tell people that when they get commit access). I'll go ahead and delete your created branches end of next week, but let me know if I should wait a bit longer with that. Thanks!

Sorry for raising an unrelated topic here, but I can't reach @Conanap directly via the mail from the git commits: @Conanap could you please create the git branches for your patches in your own Github fork instead of the main LLVM repo? LLVM's policy is to have working branches in everyone's private fork (even though I don't think we explicitly tell people that when they get commit access). I'll go ahead and delete your created branches end of next week, but let me know if I should wait a bit longer with that. Thanks!

Ah apologies, I'll do that. Sorry about that! I'll keep the rest of the branches on my personal repo; please feel free to delete these by the end of next week.

Conanap updated this revision to Diff 359681.Jul 19 2021, 12:31 AM

Removed metadata, -O3 to generate the test case

Conanap marked 2 inline comments as done.Jul 19 2021, 7:09 AM
nemanjai accepted this revision.Jul 19 2021, 9:16 AM

LGTM.

This revision is now accepted and ready to land.Jul 19 2021, 9:16 AM
This revision was landed with ongoing or failed builds.Jul 19 2021, 6:04 PM
This revision was automatically updated to reflect the committed changes.
Conanap reopened this revision.Jul 19 2021, 7:29 PM

Had to revert this as I'm seeing failures on buildbots not owned by us. The error is:

llc: error: : error: unable to get target for 'powerpc64le-unknown-linux-gnu', see --version and --triple.

Note that when testing on local machines I did not encounter this error, neither did PowerPC buildbots, so I'll need to look into it.

This revision is now accepted and ready to land.Jul 19 2021, 7:29 PM
lkail added a subscriber: lkail.Jul 20 2021, 4:23 AM
lkail added inline comments.
clang/test/CodeGen/builtins-ppc-xlcompat-check-ldarx-opt.ll
2

This looks problematic, we should not generate llc's output in clang's test.

lkail requested changes to this revision.Jul 20 2021, 4:32 AM

Looks we should put it in llvm/test/CodeGen/PowerPC.

This revision now requires changes to proceed.Jul 20 2021, 4:32 AM
Conanap updated this revision to Diff 360114.Jul 20 2021, 7:08 AM

Moved to llvm/test/CodeGen/PowerPC

Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2021, 7:08 AM
lkail accepted this revision.Jul 20 2021, 7:40 AM

LGTM, thanks!

This revision is now accepted and ready to land.Jul 20 2021, 7:40 AM
This revision was landed with ongoing or failed builds.Jul 20 2021, 12:15 PM
This revision was automatically updated to reflect the committed changes.