More robust variable handling

[cajfisher]

Functions apply_transmat and change_transmat should take gpointers instead of gints (Also tidied up a bit)

[ovhpa]

Hello again,

Making the cast from gpointer to gint inside the function is indeed more more compiler friendly. I think it is similar to the idea with p_i and implicit cast when calling the function (but definitely, your solution is more explicit).

However, note that the problem was not really here but within the optimizing stages of clang: when compiling without optimization (-O0) it did compile fine with clang, and without any warning. With higher optimization levels, optimization crashed, and clang complained about it. In the optimization stage, when clang tried to unroll the loop for(i=0;i<12;i++) it then found that p_i is a pointer. In my understanding it then try to resolve the address, in case there is more optimization. Then got address which is set with 0 (or 1,2,3,...,11) and doesn't know what to do with it. The implicit cast also didn't help ^^'

I think the "most robust" way to go is by passing a pointer that is really a pointer, ie. create a global: const gint transmat_modes[12]={0,1,2,3,4,5,6,7,8,9,10,11}; Then use it directly to pass values, in the g_signal_connect g_signal_connect(GTK_OBJECT(CEDIT.transmat[i]), "changed", GTK_SIGNAL_FUNC(change_transmat), &transmat_modes[i]); The changes in change_transmat function would be trivial: gint mode = *ptr;

What is your opinion? Can you implement such change?

Best,

[cajfisher]

Hi!

Using GINT_TO_POINTER does indeed send a pointer, so I don't think that is what is causing the problem with clang at higher optimization levels. If it were the case, then all the other instances where GINT_TO_POINTER is used would come up as errors also. (Not using GPOINTER_TO_INT in the receiver functions may well cause the compiler to stumble at higher optimizations).

According to the glib sitem "The GLib macros GPOINTER_TO_INT()https://developer.gnome.org/glib/stable/glib-Type-Conversion-Macros.html#GPOINTER-TO-INT:CAPS, GINT_TO_POINTER()https://developer.gnome.org/glib/stable/glib-Type-Conversion-Macros.html#GINT-TO-POINTER:CAPS, etc. take care to do the right thing on every platform." https://developer.gnome.org/glib/stable/glib-Type-Conversion-Macros.html

That is, as long as two conditions are met: 1) you only go from GINT_TO_POINTER then GPOINTER_TO_INT, and not GPOINTER_TO_INT then GINT_TO_POINTER, 2) the integer must be storable in 32 bits. The gdis code meets both these conditions if you use GINT_TO_POINTER, so it should work fine. The problem with p_i code is that it sets the pointer address to 0 to 12, which will cause the compiler big problems, when it is the reverse that you want (i.e., to store an integer in a single bit (pointer) whose address depends on the machine (and compiler)). Type Conversion Macros: GLib Reference Manualhttps://developer.gnome.org/glib/stable/glib-Type-Conversion-Macros.html Description. Many times GLib, GTK+, and other libraries allow you to pass "user data" to a callback, in the form of a void pointer. From time to time you want to pass an integer instead of a pointer. You could allocate an integer, with something like: 1 2. int ip = g_new (int, 1); ip = 42; But this is inconvenient, and it's annoying to have ... developer.gnome.org 

Thus, creating a new global just to pass indices to the translation matrix is not necessary.

Note that the integers 0-11 are the matrix indices passed to change_transmat. The "modes" are integers indicating how (or to what) the matrix is to be applied (currently three, AT_ANY, AT_SELECTED, and AT_LATTICE) using apply_transmat.

Regards,

C

---

差出人: Okadome Valencia @.> 送信日時: 2021年5月20日 18:30 宛先: arohl/gdis @.> CC: ｸﾚｲｸﾞ･ﾌｨｯｼｬｰ @.>; Author @.> 件名: Re: [arohl/gdis] More robust variable handling (#35)

Hello again,

Making the cast from gpointer to gint inside the function is indeed more more compiler friendly. I think it is similar to the idea with p_i and implicit cast when calling the function (but definitely, your solution is more explicit).

However, note that the problem was not really here but within the optimizing stages of clang: when compiling without optimization (-O0) it did compile fine with clang, and without any warning. With higher optimization levels, optimization crashed, and clang complained about it. In the optimization stage, when clang tried to unroll the loop for(i=0;i<12;i++) it then found that p_i is a pointer. In my understanding it then try to resolve the address, in case there is more optimization. Then got address which is set with 0 (or 1,2,3,...,11) and doesn't know what to do with it. The implicit cast also didn't help ^^'

I think the "most robust" way to go is by passing a pointer that is really a pointer, ie. create a global: const gint transmat_modes[12]={0,1,2,3,4,5,6,7,8,9,10,11}; Then use it directly to pass values, in the g_signal_connect g_signal_connect(GTK_OBJECT(CEDIT.transmat[i]), "changed", GTK_SIGNAL_FUNC(change_transmat), &transmat_modes[i]); The changes in change_transmat function would be trivial: gint mode = *ptr;

What is your opinion? Can you implement such change?

Best,

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/arohl/gdis/pull/35#issuecomment-844911337, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AP5N32CC5AWKYN4JJNYK7PDTOTJC5ANCNFSM45GJN7AA.

[ovhpa]

Hi,

I know what GLIB says, but I am still inconfortable in setting a pointer that contains an integer value, and not points to an integer value (as pointer should do). What I would prefer is to store an integer as an integer, and a pointer as a pointer, and if possible not use a trick to store an integer in or as a pointer. But that may be a little too much, due to GKT limitation. I know, ... we did this a lot in GDIS :D

So I will pull your PR. Let me some time to check ;)

---

If it were the case, then all the other instances where GINT_TO_POINTER is used would come up as errors also. (Not using GPOINTER_TO_INT in the receiver functions may well cause the compiler to stumble at higher optimizations).

The transmat case is specific because it lies on top of a loop that can be vectorized, which is not the case of any other GINT_TO_POINTER use. The pointer-containing-integer that is passed is then itself a candidate for variable vectorization.

For reference, in case something breaks due to new plateform / endianess / GLIB / clang etc, the global implementation:

@@ -61,6 +61,8 @@ extern struct elem_pak elements[];

 #define CEDIT (sysenv.cedit)

+gint transmat_index[12]={0,1,2,3,4,5,6,7,8,9,10,11};
+
 enum{AT_ANY, AT_SELECTED, AT_LATTICE};

 /*********************************************/
@@ -249,9 +251,11 @@ update_transmat();
 /********************************************/
 /* put text entry values into actual matrix */
 /********************************************/
-void change_transmat(GtkWidget *w, gint i)
+void change_transmat(GtkWidget *w, gpointer ptr)
 {
 const gchar *text;
+gint *ptr_i = ptr;
+gint i = *ptr_i;

 if(w==NULL) return;

@@ -2340,7 +2344,7 @@ reset_transmat();
 //                   GTK_SIGNAL_FUNC(change_transmat), p_i);
 for(i=0;i<12;i++)
        g_signal_connect(GTK_OBJECT(CEDIT.transmat[i]), "changed",
-                       GTK_SIGNAL_FUNC(change_transmat), GINT_TO_POINTER(i));
+                       GTK_SIGNAL_FUNC(change_transmat), &transmat_index[i]);
 }

 /***********************************************/