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

Improved support for flags #11

Open
5 tasks
hiker opened this issue Jul 26, 2024 · 2 comments
Open
5 tasks

Improved support for flags #11

hiker opened this issue Jul 26, 2024 · 2 comments

Comments

@hiker
Copy link
Owner

hiker commented Jul 26, 2024

The new compiler class introduces a separate Flag class, in addition to the existing AddFlags and FlagsConfig.

The Flags and FlagsConfig classes should be combined into one, allowing conditional (AddFlags) and unconditional flags to be used in one object. Additionally to path-specific flags, we need to:

  • Support compiler-specific flags in general (likely move flags from the steps into the compiler class, so we can have different flags for intel and gnu etc)
  • Support compilation modes: fast_debug, release etc - different sets of flags.
  • have conditionals depending on compiler version
  • combination of compiler version and path
  • Need to be able to remove flags (esp. taking OpenMP into account, which might need to work tightly with the compiler object, since the name of the openmp flag is part of the compiler tool)
@hiker hiker mentioned this issue Jul 26, 2024
5 tasks
@hiker
Copy link
Owner Author

hiker commented Jul 26, 2024

Suggested implementation details:

  • Each compiler has a Flag object (that is actually already the case ... Iirc every tool has flags), but that Flag object is just a simple list of str.
  • Each flag object provides different mode names as strings (maybe keys in a dictionary), e.g. "fast-debug", "memory-debug", "optimised". These names should be flexible (i.e. no test if a name is valid). Additionally, it must keep track of one list of 'generic' flags, i.e. flags that must be used for any mode (example: ifort should always be using the option -traceback, so this would be added as generic flag)
    • That might be a complete overkill (and can certainly wait till later): would it make sense to have a kind of hierarchy and inheritance in place? E.g. fast_debug would inherit from generic. The memory-debug would inherit from fast-debug and add memory-specific flags. And optimised might inherit from fast_debug, but add -O3 (or so). Very flexible (relying that the flags will be interpreted left to right. E.g. in the optimised example, the flag string would be -g -O0 ... -O3, with O3 being the effective flag.
  • The BuildConfig takes a mode parameter (str). We could consider a consistency check at some stage (the mode specified in the BuildConfig must indeed be defined in the compiler used)
  • The compiler will ask the flag object for the flags when compiling, the flag object should just return a list of strings. It will provide a BuildConfig object in this call, from which the mode is taken, and the input filename. It should first take the generic flags (or follow the inheritance if we actually should do that), then add the mode-specific flags. Example call: compilation_flags = self.flags.get_flags(config, input_file).
  • We need to be able to add/remove/modify(?) flags. Add is the first we need, the rest would like be more 'good to have in the future'
  • When adding flags an optional 'path condition' can be specified (Fab already has that implemented) - the flags specified would only be added if the compiled file (which was provided as parameter in the get_flags call above). Details (including implementation :) ) can be taken from the existing Fab implementation (or we could use the current implementation as a base if suitable).
  • Similarly, flags can have additional compiler-version condition, so the flag would only be added if the compiler version is in the corresponding range (most flexible implementation with least coding effort: provide optional min and max).

The implementation can be done in several smaller steps:

  1. Add Flags properly to compiler (atm compilation flags are provided in the compilation step - we should change this so that the flags are added to the compiler). For now do not remove the existing flags from compilation, since we need the path-specific capability (which we won't have yet).
  2. Add mode to the BuildConfig and compiler flags.
  3. Support path-specific flags. Then we can finally remove the flag parameters from the compilation steps.
  4. Support compiler version conditional flag.

@hiker
Copy link
Owner Author

hiker commented Jul 29, 2024

Originally, it would have been feasible to implement the flags as class/static variables (meaning if you inherit from a compiler, you would also get the flags). Just to clarify, the variables should be stored in the instances (taken from the ToolRepository). This is a requirement for the upcoming proper compiler wrapper, which allows us to do:

    gcc = Gcc()
    mpicc = Mpicc(gcc)
    assert gcc.flags == []
    assert mpicc.flags == []
    # Setting flags in gcc must become visible in the wrapper compiler:
    gcc.add_flags(["-a", "-b"])
    assert gcc.flags == ["-a", "-b"]
    assert mpicc.flags == ["-a", "-b"]

Basically, a site can just take the (say) ifort compiler from the tool repository, add whatever flags it wants to add (with all the flexibility added here). But if an app then select to use mpif90 based on ifort, it will automatically get the site-specific flags. Of if a site doesn't use mpif90, but mpiifort for intel, it can just declare mpiifort based on intel, and will also get the flags.

It would also allow an application to define more than one (say) ifort compiler with different flags ... though I can't see a good use case for that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant