From 897c14185f8a5e6506c9037d4342329a460a7a95 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Mon, 29 Jul 2019 00:38:00 +0200 Subject: [PATCH] smpi: some useless cleanups while I read this code --- src/simgrid/sg_config.cpp | 4 +++- src/smpi/internals/smpi_actor.cpp | 6 ++---- src/smpi/internals/smpi_deployment.cpp | 24 +++++++++++------------- src/smpi/internals/smpi_global.cpp | 19 ++++++++----------- 4 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/simgrid/sg_config.cpp b/src/simgrid/sg_config.cpp index 7709d360b8..1bf0f9f91c 100644 --- a/src/simgrid/sg_config.cpp +++ b/src/simgrid/sg_config.cpp @@ -354,7 +354,9 @@ void sg_config_init(int *argc, char **argv) #if HAVE_SMPI simgrid::config::declare_flag("smpi/host-speed", "Speed of the host running the simulation (in flop/s). " "Used to bench the operations.", - 20000.0); + 20000.0, [](const double& val) { + xbt_assert(val > 0.0, "Invalid value (%f) for 'smpi/host-speed': it must be positive.", val); + }); simgrid::config::alias("smpi/host-speed", {"smpi/running_power", "smpi/running-power"}); simgrid::config::declare_flag("smpi/keep-temps", "Whether we should keep the generated temporary files.", diff --git a/src/smpi/internals/smpi_actor.cpp b/src/smpi/internals/smpi_actor.cpp index 44e9d5ab27..3a5af91263 100644 --- a/src/smpi/internals/smpi_actor.cpp +++ b/src/smpi/internals/smpi_actor.cpp @@ -221,10 +221,8 @@ int ActorExt::sampling() void ActorExt::init() { - if (smpi_process_count() == 0) { - xbt_die("SimGrid was not initialized properly before entering MPI_Init. Aborting, please check compilation process " - "and use smpirun\n"); - } + xbt_assert(smpi_process_count() != 0, "SimGrid was not initialized properly before entering MPI_Init. " + "Aborting, please check compilation process and use smpirun."); simgrid::s4u::ActorPtr proc = simgrid::s4u::Actor::self(); // cheinrich: I'm not sure what the impact of the SMPI_switch_data_segment on this call is. I moved diff --git a/src/smpi/internals/smpi_deployment.cpp b/src/smpi/internals/smpi_deployment.cpp index 379c77b170..c73318220d 100644 --- a/src/smpi/internals/smpi_deployment.cpp +++ b/src/smpi/internals/smpi_deployment.cpp @@ -22,7 +22,13 @@ public: , present_processes(0) , comm_world(comm) , finalization_barrier(finalization_barrier) - { } + { + MPI_Group group = new simgrid::smpi::Group(size); + comm_world = new simgrid::smpi::Comm(group, nullptr, 0, -1); + // FIXME : using MPI_Attr_put with MPI_UNIVERSE_SIZE is forbidden and we make it a no-op (which triggers a warning + // as MPI_ERR_ARG is returned). Directly calling Comm::attr_put breaks for now, as MPI_UNIVERSE_SIZE,is <0 + // instance.comm_world->attr_put(MPI_UNIVERSE_SIZE, reinterpret_cast(instance.size)); + } const std::string name; int size; @@ -51,25 +57,17 @@ extern int process_count; // How many processes have been allocated over all ins */ void SMPI_app_instance_register(const char *name, xbt_main_func_t code, int num_processes) { - if (code != nullptr) { // When started with smpirun, we will not execute a function + if (code != nullptr) // When started with smpirun, we will not execute a function simgrid::s4u::Engine::get_instance()->register_function(name, code); - } - static int already_called = 0; + static bool already_called = false; if (not already_called) { - already_called = 1; - std::vector list = simgrid::s4u::Engine::get_instance()->get_all_hosts(); - for (auto const& host : list) { + already_called = true; + for (auto const& host : simgrid::s4u::Engine::get_instance()->get_all_hosts()) host->extension_set(new simgrid::smpi::Host(host)); - } } Instance instance(std::string(name), num_processes, MPI_COMM_NULL, new simgrid::s4u::Barrier(num_processes)); - MPI_Group group = new simgrid::smpi::Group(instance.size); - instance.comm_world = new simgrid::smpi::Comm(group, nullptr, 0, -1); -// FIXME : using MPI_Attr_put with MPI_UNIVERSE_SIZE is forbidden and we make it a no-op (which triggers a warning as MPI_ERR_ARG is returned). -// Directly calling Comm::attr_put breaks for now, as MPI_UNIVERSE_SIZE,is <0 -// instance.comm_world->attr_put(MPI_UNIVERSE_SIZE, reinterpret_cast(instance.size)); process_count+=num_processes; diff --git a/src/smpi/internals/smpi_global.cpp b/src/smpi/internals/smpi_global.cpp index 4677f6a7a6..cfec9be2af 100644 --- a/src/smpi/internals/smpi_global.cpp +++ b/src/smpi/internals/smpi_global.cpp @@ -378,8 +378,10 @@ static void smpi_init_options(){ simgrid::smpi::Colls::set_collectives(); simgrid::smpi::Colls::smpi_coll_cleanup_callback = nullptr; smpi_cpu_threshold = simgrid::config::get_value("smpi/cpu-threshold"); - smpi_host_speed = simgrid::config::get_value("smpi/host-speed"); - xbt_assert(smpi_host_speed > 0.0, "You're trying to set the host_speed to a non-positive value (given: %f)", smpi_host_speed); + if (smpi_cpu_threshold < 0) + smpi_cpu_threshold = DBL_MAX; + + smpi_host_speed = simgrid::config::get_value("smpi/host-speed"); std::string smpi_privatize_option = simgrid::config::get_value("smpi/privatization"); if (smpi_privatize_option == "no" || smpi_privatize_option == "0") smpi_privatize_global_variables = SmpiPrivStrategies::NONE; @@ -401,9 +403,6 @@ static void smpi_init_options(){ smpi_privatize_global_variables = SmpiPrivStrategies::DLOPEN; } - if (smpi_cpu_threshold < 0) - smpi_cpu_threshold = DBL_MAX; - std::string val = simgrid::config::get_value("smpi/shared-malloc"); if ((val == "yes") || (val == "1") || (val == "on") || (val == "global")) { smpi_cfg_shared_malloc = SharedMallocType::GLOBAL; @@ -564,13 +563,11 @@ static void smpi_init_privatization_dlopen(const std::string& executable) char fullpath[512] = {'\0'}; strncpy(fullpath, libname.c_str(), 511); #if not defined(__APPLE__) && not defined(__HAIKU__) - int ret = dl_iterate_phdr(visit_libs, fullpath); - if (ret == 0) - xbt_die("Can't find a linked %s - check the setting you gave to smpi/privatize-libs", fullpath); - else - XBT_DEBUG("Extra lib to privatize found : %s", fullpath); + xbt_assert(0 != dl_iterate_phdr(visit_libs, fullpath), + "Can't find a linked %s - check your settings in smpi/privatize-libs", fullpath); + XBT_DEBUG("Extra lib to privatize '%s' found", fullpath); #else - xbt_die("smpi/privatize-libs is not (yet) compatible with OSX"); + xbt_die("smpi/privatize-libs is not (yet) compatible with OSX nor with Haiku"); #endif privatize_libs_paths.push_back(fullpath); dlclose(libhandle); -- 2.20.1