Page MenuHomePhabricator

[AIX] Disable tests that fail because of no 64-bit XCOFF object file support
ClosedPublic

Authored by Jake-Egan on Nov 2 2021, 1:49 PM.

Details

Summary

The modified tests fail because 64-bit XCOFF object files are not currently supported on AIX. This patch disables these tests on 64-bit AIX for now.

This patch is similar to D111887 except the failures on this patch are on a 64-bit build.

Diff Detail

Event Timeline

Jake-Egan created this revision.Nov 2 2021, 1:49 PM
Jake-Egan requested review of this revision.Nov 2 2021, 1:49 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 2 2021, 1:49 PM
Jake-Egan edited the summary of this revision. (Show Details)Nov 2 2021, 1:50 PM
Jake-Egan added a reviewer: shchenz.
Jake-Egan updated this revision to Diff 384220.Nov 2 2021, 1:56 PM

Fix llvm/test/DebugInfo/omit-empty.ll.

shchenz added a reviewer: jsji.Nov 3 2021, 7:08 AM

This seems not right. Now we disable too many debug cases for object mode even for 32bit target. Maybe we should test clang (built with --default-target-triple=powerpc64-ibm-aix) after our backend support 64-bit object mode(-filetype=obj)

Jake-Egan updated this revision to Diff 384521.Nov 3 2021, 10:56 AM

Updated patch to XFAIL only on 64-bit.

Jake-Egan edited the summary of this revision. (Show Details)Nov 3 2021, 10:57 AM
Jake-Egan updated this revision to Diff 385830.Nov 9 2021, 7:57 AM

Add LLVM :: DebugInfo/attr-btf_type_tag.ll

jsji added a comment.Nov 18 2021, 7:05 AM

Can we use UNSUPPORTED instead of XFAIL since it is unsupported?

Can we use UNSUPPORTED instead of XFAIL since it is unsupported?

If 64-bit XCOFF object files will be supported in the future, I think it makes more sense to use XFAIL because these tests will still be run and pass after implementation.

lkail added a subscriber: lkail.Nov 18 2021, 7:22 AM

Is there any way to filter these tests out on AIX in lit.local.cfg?

lkail added a reviewer: Restricted Project.Nov 18 2021, 7:25 AM
jsji added a comment.Nov 18 2021, 7:28 AM

Can we use UNSUPPORTED instead of XFAIL since it is unsupported?

If 64-bit XCOFF object files will be supported in the future, I think it makes more sense to use XFAIL because these tests will still be run and pass after implementation.

We should remove the UNSUPPORTED when we enable the 64-bit XCOFF object file support . It is a waste of machine time to run them *NOW*, especially considering the number of these failing tests.

jsji added a comment.Nov 18 2021, 7:29 AM

Is there any way to filter these tests out on AIX in lit.local.cfg?

Agree, I would prefer we do something similar to https://reviews.llvm.org/rG666accf283311c5110ae4e2e5e4c4b99078eed15#change-NFfZJdfkKBjR to exclude the unsupported files for now.

Jake-Egan updated this revision to Diff 389196.Nov 23 2021, 7:16 AM

Thanks for the review. I updated the patch to use lit.cfg.py to filter tests that use obj options. For tests that don't use the option or has an individual folder, I changed them to UNSUPPORTED instead of XFAIL.

jsji added a comment.Nov 23 2021, 7:33 AM

Thanks, looks much better now.

clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
2

there is no fmodule-format=obj here, why are we failing here?

clang/test/lit.cfg.py
262

Why not adding ASTMerge and its subfolders?

Jake-Egan added inline comments.Nov 25 2021, 10:53 AM
clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
2

They have the same error as the other tests:

64-bit XCOFF object files are not supported yet.
clang/test/lit.cfg.py
262

Adding those folders were causing regressions:

Clang :: utils/update_cc_test_checks/global-hex-value-regex.test
Clang :: utils/update_cc_test_checks/global-value-regex.test

With error:

did not discover any tests for provided path(s)

I think because the only test in the folder gets excluded, so LIT can't find any test to run.

I could create a lit.local.cfg in each ASTMerge subfolder to unsupport each one if that's preferred.

Removed some tests already disabled by D114481.

shchenz added inline comments.Dec 15 2021, 5:20 PM
clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
2

Which RUN line causes the 64-bit XCOFF object files are not supported yet assertion? NO -emit-obj or fmodule-format=obj found in all the RUN lines.

clang/test/lit.cfg.py
256

Can we add -fintegrated-as here too?

Added -fintegrated-as option.

Jake-Egan marked an inline comment as done.Tue, Dec 21, 11:02 AM
Jake-Egan added inline comments.
clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
2
// RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format experimental-full \
// RUN:   -mode preprocess-minimized-sources -module-name=header1 >> %t.result

It fails here. t.result contains a module using the -fmodule-format=obj option.

shchenz added inline comments.Tue, Dec 21, 6:38 PM
clang/test/ASTMerge/anonymous-fields/test.cpp
1 ↗(On Diff #395724)

Do we still need this? -emit-obj is already excluded?

clang/test/ASTMerge/codegen-body/test.c
2

Do we still need this? -emit-obj is already excluded?

clang/test/ASTMerge/injected-class-name-decl/test.cpp
1 ↗(On Diff #395724)

Do we still need this? -emit-obj is already excluded?

clang/test/ClangScanDeps/modules-full-by-mod-name.cpp
17

typos?

Fixed typo.

Jake-Egan marked an inline comment as done.Wed, Dec 22, 11:05 AM
Jake-Egan added inline comments.
clang/test/ASTMerge/codegen-body/test.c
2

Excluding this test causes regressions because LIT can't find any tests at /codegen-body directory:

Clang :: utils/update_cc_test_checks/global-hex-value-regex.test
Clang :: utils/update_cc_test_checks/global-value-regex.test

I figure it's easier to mark this test unsupported.

Look almost good! Thanks for doing this.

clang/test/ASTMerge/codegen-body/test.c
2

Is it possible to add expected-no-diagnostics for any of the above two RUN lines? So we will have a test point and be able to exclude this case?

Jake-Egan added inline comments.Tue, Jan 4, 10:08 AM
clang/test/ASTMerge/codegen-body/test.c
2

Unfortunately adding expected-no-diagnostics fix the issue. The existing expected-no-diagnostics is already applied to all the RUN lines.

Jake-Egan added inline comments.Thu, Jan 6, 6:43 AM
clang/test/ASTMerge/codegen-body/test.c
2

fix the issue

I meant *doesn't* fix the issue.

Jake-Egan edited the summary of this revision. (Show Details)Thu, Jan 6, 10:30 AM
shchenz accepted this revision.Thu, Jan 6, 5:21 PM

LGTM. Thanks for the explanation.

This revision is now accepted and ready to land.Thu, Jan 6, 5:21 PM
This revision was landed with ongoing or failed builds.Sun, Jan 9, 9:26 AM
This revision was automatically updated to reflect the committed changes.