[PATCH] D137725: [OpenMP][OMPIRBuilder] Mirgrate getName from clang to OMPIRBuilder

[Jan Sjödin via Phabricator via llvm-commits]

jsjodin added a comment.

In D137725#3921567 <https://reviews.llvm.org/D137725#3921567>, @jsjodin wrote:

> In D137725#3919652 <https://reviews.llvm.org/D137725#3919652>, @jdoerfert wrote:
>
>> Maybe we want a configuration object that is populated once by the user and passed to the OMPIRBuilder to cluster all such configurations, e.g., device, embedded, etc. We can also choose separators based on that internally.
>
> Yes, I think that sounds good. Formalizing the attributes that affect codgen (in a configuration object) should make the code easier to understand/maintain. I can start working on a patch to add the configuration object.

After looking a bit more closely into this, there are a few factors to consider. There are uses of the OpenMPIRBuilder, for example in mlir with the OpenACC frontend (it uses the OpenMPIRBuilder for some reason) and in the OMP optimizer, where I do not know if the configuration makes sense. The OffloadEntriesInfoManager is dependent on these flags, which means that the configuration must be available somehow. 
I am working on creating the new class OpenMPIRBuilderConfig (or maybe OpenMPIRBuilderContext). The config can be passed in to both the OpenMPIRBuilder and OffloadEntriesInfoManager, constructors. The config can be made optional either by using a pointer which could be null so that the uses of OpenMPIRBuilder in mlir and opt do not have to create a config, howerver using functions that require it would cause an error (segfault). Alternatively we could make the various flags in the Config into llvm::Optional<Flag> so that the configuration can be done flexibly, and use of any functions in the Builder would give a nicer error if the value is not set. It will also make the code look more uniform with "Config." instead of "Config->".

I'm currently working on using llvm::Optional for the various attiibutes, since that is more flexible. Let me know if you have any other ideas how to handle this.


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

https://reviews.llvm.org/D137725