[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]
----------------
lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > I would not rely on the predetermined indices here, better to use some kind of patterns here just like in other places.
> > > > > Could you give an example about what you suggest? For instance, some other tests I should look into.
> > > > Just like in this test when you're using vars.
> > > Sorry I was not clear before. What do you mean by "predetermined indices" here? If you are referring to, for example, `%0` in `store i8* %0, i8** [[HANDLEADDR:%[^,]+]]`, I guess there is no way to get rid of `%0` because it means the first argument of the function?
> > Yes, I meant those `%0` like registers. Better to mark them as variables in function declaration and use those names in the checks.
> Now it's like `define {{.*}}void @.omp_mapper.{{.*}}C.id{{.*}}(i8*, i8*, i8*, i64, i64)`, I think you are suggesting something like `define {{.*}}void @.omp_mapper.{{.*}}C.id{{.*}}(i8* [[HANDLE:%[^,]+]], i8* [[BPTR:%[^,]+]], ...)`, and later I can use `store i8* [[HANDLE]], i8** [[HANDLEADDR:%[^,]+]]`
> 
> I'm not sure how to add names for function arguments. They seems to be always nameless like `(i8*, i8*, i8*, i64, i64)`. Is there a way to do that?
If the clang parameters have names, the llvm params also will get the names. But it is not worth it to add the names to the function. Could just use regexp here to avoid using LLVM register names? Just `%{{·+}}`. And rely on the order, i.e. remove `-DAG` checks?


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

https://reviews.llvm.org/D59474