This is an archive of the discontinued LLVM Phabricator instance.

[NFC][OpenMP] Use clang_cc1 to driver tests
ClosedPublic

Authored by jsji on Sep 3 2021, 12:40 PM.

Details

Summary

The test driver-fopenmp-extensions.c is failing on platforms that does
not use integrated-as. It can be reproduced using -fno-integrated-as on
Linux too.

bin/clang -c -Xclang -verify=omp -fopenmp -fopenmp-extensions
-fno-openmp-extensions
../llvm-project/clang/test/OpenMP/driver-fopenmp-extensions.c
-fno-integrated-as
Assembler messages:
Error: can't open /tmp/driver-fopenmp-extensions-8fafe8.s for reading:
No such file or directory
clang-14: error: assembler command failed with exit code 1 (use -v to
see invocation)

The goal of this test is to verify syntax diags only,
so we should use clang_cc1 to test.

Diff Detail

Event Timeline

jsji created this revision.Sep 3 2021, 12:40 PM
jsji requested review of this revision.Sep 3 2021, 12:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2021, 12:40 PM

I think we can use just clang_cc1 here instead of %clang -c

jsji updated this revision to Diff 370653.Sep 3 2021, 12:47 PM

Use clang_cc1 instead.

jsji added a comment.Sep 3 2021, 12:48 PM

I think we can use just clang_cc1 here instead of %clang -c

Good idea.

This revision is now accepted and ready to land.Sep 3 2021, 12:53 PM

This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option.

jsji added a comment.Sep 3 2021, 12:57 PM

This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option.

OK, I can revert to driver + fsyntax-only if that is preferred?

This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option.

OK, I can revert to driver + fsyntax-only if that is preferred?

That would be my preference. Otherwise, we never test the interface exposed to the user, and the test is misnamed.

jsji updated this revision to Diff 370658.Sep 3 2021, 1:03 PM

Use driver + -fsyntax-only.

This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option.

Better not to do such checks, just need to check that driver correctly translates driver options to the frontend options (using -###), better to use -verify only with the frontend tests. Suggest to rework this test.

jsji added a comment.Sep 3 2021, 1:04 PM

@ABataev Is this still OK for you?

jsji added a comment.EditedSep 3 2021, 1:07 PM

This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option.

Better not to do such checks, just need to check that driver correctly translates driver options to the frontend options (using -###), better to use -verify only with the frontend tests. Suggest to rework this test.

Make sense to me. How about we use _cc1 as Alexey suggested here (and rename the file to remove driver- prefix). Maybe Joel can add another test to test the driver correctly translates driver options to the frontend options (using -###) in test/Driver dir?

jsji updated this revision to Diff 370663.Sep 3 2021, 1:24 PM

use _cc1 for now.

jdenny added a comment.Sep 3 2021, 1:26 PM

This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option.

Better not to do such checks, just need to check that driver correctly translates driver options to the frontend options (using -###), better to use -verify only with the frontend tests. Suggest to rework this test.

Make sense to me. How about we use _cc1 as Alexey suggested here (and rename the file to remove driver- prefix).

OK.

Maybe Joel can add another test to test the driver correctly translates driver options to the frontend options (using -###) in test/Driver dir?

I probably won't get to it today, but I can work on it later.

jsji added a comment.Sep 3 2021, 1:28 PM

I probably won't get to it today, but I can work on it later.

Sure, take your time. Thank you!

jdenny accepted this revision.Sep 3 2021, 1:30 PM

LGTM. Thanks for the fix.

jsji retitled this revision from [NFC][OpenMP] Add fsyntax-only to driver tests to [NFC][OpenMP] Use clang_cc1 to driver tests.Sep 3 2021, 1:33 PM
jsji edited the summary of this revision. (Show Details)
This revision was landed with ongoing or failed builds.Sep 3 2021, 1:34 PM
This revision was automatically updated to reflect the committed changes.