[Openmp-commits] [PATCH] D158544: [OpenMP] Optimized trivial multiple edges from task dependency graph

Joachim Protze via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Aug 24 10:02:06 PDT 2023


protze.joachim added a comment.

The new tests are missing a `RUN` line (see first line in below diff).
I think, you should also add `REQUIRES: linux` to avoid failures on Windows from missing dll-exports for your newly added kmpc functions

I furthermore suggest to add a task with `depend(out:y)` to the inoutset test just to verify that dropping the edges does not introduce inconsistencies:

  0a1,2
  > // RUN: %libomp-compile-and-run
  > // REQUIRES: linux
  17,19c19,21
  <       kmp_task_t *A, *B, *C, *D, *E, *F;
  <       kmp_depnode_list_t *A_succ, *B_succ, *C_succ;
  <       kmp_base_depnode_t *D_node, *E_node, *F_node;
  ---
  >       kmp_task_t *A, *B, *C, *D, *E, *F, *G;
  >       kmp_depnode_list_t *A_succ, *B_succ, *C_succ, *E_succ, *F_succ;
  >       kmp_base_depnode_t *D_node, *E_node, *F_node, *G_node;
  60a63,67
  >       deps[1].flags = 2; // OUT
  >       G = __kmpc_omp_task_alloc(&loc, gtid, TIED, sizeof(kmp_task_t), 0, NULL);
  >       __kmpc_omp_task_with_deps(&loc, gtid, G, 1, deps + 1, 0, 0);
  >       
  64a71,72
  >       E_succ = __kmpc_task_get_successors(E);
  >       F_succ = __kmpc_task_get_successors(F);
  68a77
  >       G_node = __kmpc_task_get_depnode(G);
  101a111,114
  > 
  >       // E -> G and F -> G
  >       assert(E_succ && !E_succ->next && F_succ && !F_succ->next);
  >       assert(E_succ->node == G_node && F_succ->node == G_node);

Indention doesn't look right for the newly added if blocks in `kmp_taskdeps.cpp`. `git clang-format` doesnt't help, so please add indention for these blocks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158544/new/

https://reviews.llvm.org/D158544



More information about the Openmp-commits mailing list