[PATCH] D71267: [OpenMPIRBuilder] Add support for generating kmpc_for_static_fini

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2819
+  }
 }
 
----------------
jdoerfert wrote:
> kiranchandramohan wrote:
> > jdoerfert wrote:
> > > We do emit exactly the same code in both cases, don't we?
> > > If so, use the always available OMPIRBuilder unconditionally.
> > As of now there is a minor difference. The IRBuilder static fini generates a new call to kmpc_global_thread_num, the existing flow reuses the value computed by a previous call to kmpc_global_thread_num. Seems to be the same functionality.
> > ```
> > <   call void @__kmpc_for_static_fini(%struct.ident_t* @1, i32 %8)
> > ---
> > >   %31 = call i32 @__kmpc_global_thread_num(%struct.ident_t* @2)
> > >   call void @__kmpc_for_static_fini(%struct.ident_t* @1, i32 %31)
> > ```
> ok, do you think you can update the tests here by hand? That would allow to remove the old handling right away.
> 
I will try.


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:92
+bool isOpenMPLoopDirective(Directive DKind);
+
 } // end namespace omp
----------------
jdoerfert wrote:
> kiranchandramohan wrote:
> > jdoerfert wrote:
> > > Where is the definition for these?
> > Good catch. These are in the OMPConstants.cpp file. But i guess we do not want that file.
> > 
> > So we can generate these from the tablegen and remove their declaration here and in OMPConstants.cpp. Is that the right approach?
> I think the tablegen way is good *if* we can make it nice. If not, we can keep the constants.cpp.
> Nice would be, for example, if we generate the `isOpenMPNestingDistributeDirective` by checking if the directive name starts with `distribute`. `isOpenMPDistributeDirective` is similar, `isOpenMPLoopDirective` will probably require a flag in the directives, though that seems ok and useful either way.
> 
> WDYT?
I was thinking of having these functions as properties and in each directive we can say whether this directive has the property. The property can also be classified (if necessary) as conjunctive or disjunctive and based on that we can generate the function.
Looping through all the directives we can generate these functions. Example given below.

def isOpenMPLoopDirective : Property {
   let type = Disjunctive;
}

def OMP_TaskLoop : Directive<"taskloop"> {
  let allowedClauses = [
    ...
  ];
  let properties = [isOpenMPLoopDirective];
  ...
}

def OMP_For : Directive<"for"> {
  let allowedClauses = [
    ...
  ];
  let properties = [isOpenMPLoopDirective];
}


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

https://reviews.llvm.org/D71267