This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Fix: opposite attributes could be set by -fno-inline
ClosedPublic

Authored by igor.kirillov on Oct 27 2021, 11:46 AM.

Details

Summary

After the changes introduced by D106799 it is possible to tag
outlined function with both AlwaysInline and NoInline attributes using
-fno-inline command line options.
This issue is similiar to D107649.

Diff Detail

Event Timeline

igor.kirillov created this revision.Oct 27 2021, 11:46 AM
igor.kirillov requested review of this revision.Oct 27 2021, 11:46 AM
Herald added a project: Restricted Project. · View Herald Transcript
igor.kirillov added inline comments.Oct 27 2021, 11:55 AM
clang/lib/CodeGen/CGStmtOpenMP.cpp
5370

I noticed that here we set this attribute on the same function that is created by emitOutlinedFunctionPrologue but there AlwaysInline attribute is guaranteed to be set under same condition. So I suppose we could remove this piece of code?

jhuber6 accepted this revision.Nov 1 2021, 6:49 AM

LGTM.

clang/lib/CodeGen/CGStmtOpenMP.cpp
5370

Yeah, you can remove this.

clang/test/OpenMP/parallel_for_noinline.cpp
1

This can probably be merged with the other test added in D107649 since it's the same problem, just add another run line and run the update script.

This revision is now accepted and ready to land.Nov 1 2021, 6:49 AM
igor.kirillov added inline comments.Nov 1 2021, 11:54 AM
clang/test/OpenMP/parallel_for_noinline.cpp
1

It was my first thought too but I don't see how to check this bug with #pragma omp parallel if(0). If I add function with #pragma omp parallel for there it goes contrary to the test file name (parallel_if_codegen_PR51349). More than that, checks generated by update_cc_test_checks.py are too overwhelming and hide whatever we were going to test there. I can do it this way or may be you have any other ideas?

What if I rename my file this test file name to parallel_for_codegen_PR51349.cpp instead? They are related and thus we don't loose sight of them.

jhuber6 added inline comments.Nov 1 2021, 12:00 PM
clang/test/OpenMP/parallel_for_noinline.cpp
1

Just adding this triggered the bug for me and hardly generates anything.

#pragma omp parallel
  ;
igor.kirillov marked an inline comment as not done.

Update test, remove redundant code

Remove my old test for the change

igor.kirillov marked an inline comment as done.Nov 2 2021, 8:11 AM
This revision was landed with ongoing or failed builds.Nov 10 2021, 8:56 AM
This revision was automatically updated to reflect the committed changes.