Skip to content
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

Regent: Detect how many dimensions Legion was compiled with #466

Closed
manopapad opened this issue Jan 11, 2019 · 9 comments
Closed

Regent: Detect how many dimensions Legion was compiled with #466

manopapad opened this issue Jan 11, 2019 · 9 comments
Assignees
Labels
Build Issues pertaining to build systems enhancement planned Feature/fix to be actively worked on - needs release target Regent Issues pertaining to Regent
Milestone

Comments

@manopapad
Copy link
Contributor

To properly interface with Legion's newly implemented support for a variable number of index space dimensions, Regent needs to know the value of the LEGION_MAX_DIM constant that Legion was compiled against when it calls terralib.includec on legion_c.h.

One option would be to autodetect this by inspecting libregent.so, the way it's done for CUDA and OpenMP, and add an -f flag so the user can set it manually when auto-detection isn't available (and do a dynamic consistency check, when the code starts running).

A cleaner option might be to have the Makefile generate a legion_defines.h header, like the one generated by CMake. This has also been suggested as a way to communicate the link flags that Legion's runtime.mk infers to the compiler (see #341).

@elliottslaughter
Copy link
Contributor

As a workaround I've added a flag -flegion-dim that you can use to tell Regent about the value of LEGION_MAX_DIM.

@lightsighter
Copy link
Contributor

Would it help if you had a runtime call to query this value from the runtime?

@elliottslaughter
Copy link
Contributor

elliottslaughter commented Jan 17, 2019

Unfortunately no, any autodetect mechanism has to work without actually loading the runtime because:

(a) on platforms like Summit we just can't do that right now

(b) even then we need to set LEGION_MAX_DIM at the point where we include the Legion header file. So if you need to load Legion in order to include that header, you've got a circular dependency.

In my opinion the most promising approach is to have legion_defines.h be generated as part of the build in both Make and CMake. That way any build-influencing variables can always be depended on being set, which is the simplest way for users (most won't even have to think about this at all) and makes this future proof as well.

@lightsighter
Copy link
Contributor

What about a static method on the runtime so you can call it without having actually started the runtime?

@elliottslaughter
Copy link
Contributor

No, we can't even load libregent.so because the ffi implementation in Lua is incomplete (and LuaJIT doesn't support PPC64le). Anything that requires calling a C or Terra function during compilation, or even just loading a shared library, is not going to work.

Futhermore, the static method still doesn't resolve the cyclic dependency, because presumably the way I call the static method is by running terralib.includec("legion.h") and the set of defines is already fixed at that point.

@lightsighter lightsighter added Regent Issues pertaining to Regent Build Issues pertaining to build systems labels Apr 23, 2019
@streichler
Copy link
Contributor

@elliottslaughter why can't terra just include legion_c.h with a large LEGION_MAX_DIM and then check afterwards if the symbols aren't actually in the .so file?

@elliottslaughter
Copy link
Contributor

This doesn't work because LEGION_MAX_DIM affects the size of domains/domain points so it's not just a matter of ignoring symbols that are not defined.

@lightsighter had a partial patch for this but it's currently disabled because it is very inefficient in the current implementation.

@elliottslaughter
Copy link
Contributor

To be clear, the partial patch is for #496 .

@elliottslaughter elliottslaughter added enhancement planned Feature/fix to be actively worked on - needs release target labels Sep 25, 2019
@elliottslaughter elliottslaughter added this to the 19.12 milestone Sep 25, 2019
@elliottslaughter
Copy link
Contributor

As of ff00fb8 we generate legion_defines.h which includes the value for LEGION_MAX_DIM so that it no longer needs to be set on the command line. -flegion-dim is no longer required or supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Issues pertaining to build systems enhancement planned Feature/fix to be actively worked on - needs release target Regent Issues pertaining to Regent
Projects
None yet
Development

No branches or pull requests

4 participants