Merge pull request #16737 from bradcray/chpl_nodeID_nonvar

Convert `chpl_nodeID` from `extern var` to `extern proc`

[reviewed by @mppf]

Declaring `chpl_nodeID` as a module-scope extern variable pointed to
an issue in which we weren't introducing a shadow variable for it as
we do for other module-scope variables.  However, it also shows a
weakness of using a module-scope extern variable for an external value
that is different on each locale: it suggests to Chapel that there's a
single extern variable when in fact there's a per-locale variable with
a distinct value on each locale.  This suggests that an extern var
isn't the best way to represent such values in Chapel.

In this PR, I change chpl_nodeID to an extern (paren-less) proc so
that the Chapel code which refers to it can remain unchanged; and I
assign it a new name in the C code so that we can implement it as a
static inline function without modifying other C code that refers to
the C-level variable.  The result is a minimal change to our code that
avoids the mismatch between describing such a value as a global
variable in Chapel that should result in a nearly identical
execution-time implementation.

Resolves #16716.

modules/internal/LocaleModelHelpSetup.chpl

@@ -39,7 +39,8 @@ module LocaleModelHelpSetup {
 
   config param debugLocaleModel = false;
 
-  extern var chpl_nodeID: chpl_nodeID_t;
+  pragma "fn synchronization free"
+  extern "get_chpl_nodeID" proc chpl_nodeID: chpl_nodeID_t;
 
   record chpl_root_locale_accum {
     var nPUsPhysAcc: atomic int;

runtime/include/chpl-comm.h

@@ -41,6 +41,14 @@ extern c_nodeid_t chpl_nodeID; // unique ID for each node: 0, 1, 2, ...
 // the current task is running.
 // Note also that this value is set only in chpl_comm_init to a value which is
 // (hopefully) unique to the running image, and never changed again.
+
+//
+// Helper function for Chapel to get the value of chpl_nodeID
+//
+static inline c_nodeid_t get_chpl_nodeID(void) {
+  return chpl_nodeID;
+}
+
 extern int32_t chpl_numNodes; // number of nodes
 
 size_t chpl_comm_getenvMaxHeapSize(void);

test/localeModels/dmk/locale_name.chpl

@@ -1,5 +1,4 @@
 extern proc chpl_nodeName(): c_string;
-extern var chpl_nodeID: chpl_nodeID_t;
 
 var locale_name = here.chpl_name();
 

test/localeModels/nodeIDbug.chpl

@@ -0,0 +1,23 @@
+var s$: sync int = 0;
+
+writeln("coforall: ");
+coforall loc in Locales {
+  on loc {
+    while (s$.readFF() != here.id) { }
+    s$.readFE();
+    writeln("chpl_nodeID: ", chpl_nodeID, ", here.id = ", here.id);
+    s$.writeEF(here.id+1);
+  }
+}
+writeln();
+s$.writeXF(0);
+
+writeln("forall: ");
+forall loc in Locales {
+  on loc {
+    while (s$.readFF() != here.id) { }
+    s$.readFE();
+    writeln("chpl_nodeID: ", chpl_nodeID, ", here.id = ", here.id);
+    s$.writeEF(here.id+1);
+  }
+}

test/localeModels/nodeIDbug.comm-none.good

@@ -0,0 +1,5 @@
+coforall: 
+chpl_nodeID: 0, here.id = 0
+
+forall: 
+chpl_nodeID: 0, here.id = 0

test/localeModels/nodeIDbug.good

@@ -0,0 +1,11 @@
+coforall: 
+chpl_nodeID: 0, here.id = 0
+chpl_nodeID: 1, here.id = 1
+chpl_nodeID: 2, here.id = 2
+chpl_nodeID: 3, here.id = 3
+
+forall: 
+chpl_nodeID: 0, here.id = 0
+chpl_nodeID: 1, here.id = 1
+chpl_nodeID: 2, here.id = 2
+chpl_nodeID: 3, here.id = 3

test/localeModels/nodeIDbug.numlocales

@@ -0,0 +1 @@
+4