Skip to content

[clang][OpenMP][NFC] Move 'allocate' clause modifier parsing into fun… #115775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

ddpagan
Copy link
Contributor

@ddpagan ddpagan commented Nov 11, 2024

…ction

Parsing of 'allocate' clause modifier ('allocator') has been moved into a separate function in anticipation of adding another modifier ('align').

…ction

Parsing of 'allocate' clause modifier ('allocator') has been moved into
a separate function in anticipation of adding another modifier ('align').
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang labels Nov 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2024

@llvm/pr-subscribers-clang

Author: David Pagan (ddpagan)

Changes

…ction

Parsing of 'allocate' clause modifier ('allocator') has been moved into a separate function in anticipation of adding another modifier ('align').


Full diff: https://github.com/llvm/llvm-project/pull/115775.diff

1 Files Affected:

  • (modified) clang/lib/Parse/ParseOpenMP.cpp (+31-17)
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 59a33eafa6be4f..c253133f611b0b 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -4519,6 +4519,36 @@ static bool parseStepSize(Parser &P, SemaOpenMP::OpenMPVarListDataTy &Data,
   return false;
 }
 
+/// Parse 'allocate' clause modifiers.
+///   If allocator-modifier exists, return an expression for it and set
+///   Data field noting modifier was specified.
+///
+static ExprResult
+parseOpenMPAllocateClauseModifiers(Parser &P, OpenMPClauseKind Kind,
+                                   SemaOpenMP::OpenMPVarListDataTy &Data) {
+  const Token &Tok = P.getCurToken();
+  Preprocessor &PP = P.getPreprocessor();
+  ExprResult Tail;
+  auto Modifier = static_cast<OpenMPAllocateClauseModifier>(
+      getOpenMPSimpleClauseType(Kind, PP.getSpelling(Tok), P.getLangOpts()));
+  if (Modifier == OMPC_ALLOCATE_allocator) {
+    Data.AllocClauseModifier = Modifier;
+    P.ConsumeToken();
+    BalancedDelimiterTracker AllocateT(P, tok::l_paren,
+                                       tok::annot_pragma_openmp_end);
+    if (Tok.is(tok::l_paren)) {
+      AllocateT.consumeOpen();
+      Tail = P.ParseAssignmentExpression();
+      AllocateT.consumeClose();
+    } else {
+      P.Diag(Tok, diag::err_expected) << tok::l_paren;
+    }
+  } else {
+    Tail = P.ParseAssignmentExpression();
+  }
+  return Tail;
+}
+
 /// Parses clauses with list.
 bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind,
                                 OpenMPClauseKind Kind,
@@ -4800,23 +4830,7 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind,
     // iterator(iterators-definition)
     ExprResult Tail;
     if (Kind == OMPC_allocate) {
-      auto Modifier = static_cast<OpenMPAllocateClauseModifier>(
-          getOpenMPSimpleClauseType(Kind, PP.getSpelling(Tok), getLangOpts()));
-      if (Modifier == OMPC_ALLOCATE_allocator) {
-        Data.AllocClauseModifier = Modifier;
-        ConsumeToken();
-        BalancedDelimiterTracker AllocateT(*this, tok::l_paren,
-                                           tok::annot_pragma_openmp_end);
-        if (Tok.is(tok::l_paren)) {
-          AllocateT.consumeOpen();
-          Tail = ParseAssignmentExpression();
-          AllocateT.consumeClose();
-        } else {
-          Diag(Tok, diag::err_expected) << tok::l_paren;
-        }
-      } else {
-        Tail = ParseAssignmentExpression();
-      }
+      Tail = parseOpenMPAllocateClauseModifiers(*this, Kind, Data);
     } else {
       HasIterator = true;
       EnterScope(Scope::OpenMPDirectiveScope | Scope::DeclScope);

@ddpagan ddpagan merged commit e8c4842 into llvm:main Nov 12, 2024
10 of 11 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 12, 2024

LLVM Buildbot has detected a new failure on builder libc-x86_64-debian-gcc-fullbuild-dbg running on libc-x86_64-debian-fullbuild while building clang at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/10173

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[       OK ] LlvmLibcFStatTest.NonExistentFile (4 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[955/1099] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysStatvfsTest.StatvfsBasic
[       OK ] LlvmLibcSysStatvfsTest.StatvfsBasic (21 us)
[ RUN      ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath
[       OK ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath (263 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[956/1099] Running unit test libc.test.src.sys.statvfs.linux.fstatvfs_test
FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.fstatvfs_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.fstatvfs_test 
cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/libc.test.src.sys.statvfs.linux.fstatvfs_test.__build__
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysFStatvfsTest.FStatvfsBasic
[       OK ] LlvmLibcSysFStatvfsTest.FStatvfsBasic (26 us)
[ RUN      ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/llvm-project/libc/test/src/sys/statvfs/linux/fstatvfs_test.cpp:39: FAILURE
Failed to match LIBC_NAMESPACE::mkdirat(AT_FDCWD, TEST_DIR, S_IRWXU) against Succeeds(0).
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "File exists".
[  FAILED  ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath
Ran 2 tests.  PASS: 1  FAIL: 1
[957/1099] Running unit test libc.test.src.sys.utsname.uname_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcUnameTest.GetMachineName
[       OK ] LlvmLibcUnameTest.GetMachineName (63 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[958/1099] Running unit test libc.test.src.sys.wait.wait4_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcwait4Test.NoHangTest
[       OK ] LlvmLibcwait4Test.NoHangTest (6 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[959/1099] Running unit test libc.test.src.sys.wait.waitpid_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcWaitPidTest.NoHangTest
[       OK ] LlvmLibcWaitPidTest.NoHangTest (4 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[960/1099] Running unit test libc.test.src.sys.epoll.linux.epoll_ctl_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcEpollCtlTest.Basic
[       OK ] LlvmLibcEpollCtlTest.Basic (64 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[961/1099] Running unit test libc.test.src.sys.prctl.linux.prctl_test
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysPrctlTest.GetSetName
[       OK ] LlvmLibcSysPrctlTest.GetSetName (44 us)
[ RUN      ] LlvmLibcSysPrctlTest.GetTHPDisable
[       OK ] LlvmLibcSysPrctlTest.GetTHPDisable (40 us)
Ran 2 tests.  PASS: 2  FAIL: 0

@ddpagan ddpagan deleted the alloc-clause-nfc branch November 12, 2024 14:29
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
llvm#115775)

…ction

Parsing of 'allocate' clause modifier ('allocator') has been moved into
a separate function in anticipation of adding another modifier
('align').
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants