On Tue, Jun 24, 2014 at 7:05 PM, Per Hedbor () @ Pike (-) developers forum 10353@lyskom.lysator.liu.se wrote:
Could you give this version of SET_SVAL a try, perhaps?
#define SET_SVAL(SVAL, TYPE, SUBTYPE, FIELD, EXPR) do { \ /* Set the type afterwards to avoid a clobbered \ * svalue in case EXPR throws. */ \ struct svalue * _sv_ptr=&( SVAL ); \ _sv_ptr->u.FIELD = (EXPR); \ *((ptrdiff_t*)&_sv_ptr->type) = TYPE_SUBTYPE(TYPE,SUBTYPE);\ } while(0)
Has exactly the same result.
It's entirely possible I'm misdiagnosing this. To come to the conclusion I did, I took the current 8.0 branch, reverted that one commit, and then began applying it piece by piece. The change to configure.in didn't break anything, the change to svalue.h did. Within that, changing from INT16 to short had no effect (on my system, I believe INT16 compiles to short anyway); adding structures and unused defines, unsurprisingly, didn't break anything; and changing the definition of SET_SVAL induced the error message. That's all the info I have to offer.
I'm starting up a completely fresh Pike clone to see if somehow I've accumulated cruft of some sort. If that behaves differently, I'll let you know.
ChrisA
On Tue, Jun 24, 2014 at 7:54 PM, Chris Angelico rosuav@gmail.com wrote:
I'm starting up a completely fresh Pike clone to see if somehow I've accumulated cruft of some sort. If that behaves differently, I'll let you know.
That didn't solve anything, but the latest commit on 8.0 branch did. Thanks Grubba! Looking good.
ChrisA
The casting through union is strictly still undefined but it works in gcc. Unfortunately, that is not the case for all compilers. Apparently Sun CC will not generate the intended code. See:
http://blog.qt.digia.com/blog/2011/06/10/type-punning-and-strict-aliasing/
A simpler solution, which doesnt need any changes to the svalue struct would have been to do the write using memcpy, which would be optimized into one pointer size store. It is defined behavior which reflects the aliasing correctly and requires only minimal modification to the original code.
Arne
On Wed, 25 Jun 2014, Chris Angelico wrote:
On Tue, Jun 24, 2014 at 7:54 PM, Chris Angelico rosuav@gmail.com wrote:
I'm starting up a completely fresh Pike clone to see if somehow I've accumulated cruft of some sort. If that behaves differently, I'll let you know.
That didn't solve anything, but the latest commit on 8.0 branch did. Thanks Grubba! Looking good.
ChrisA
Isn't also TYPE_SUBTYPE wrong in the big endian case? It seems to me that on a 64bit big endian machine both subtype and type would always end up being 0.
like so?
#define TYPE_SUBTYPE(X,Y) (((Y)|((X)<<16)) << (sizeof(ptrdiff_t) - 32))
Arne
On Tue, 24 Jun 2014, Arne Goedeke wrote:
The casting through union is strictly still undefined but it works in gcc. Unfortunately, that is not the case for all compilers. Apparently Sun CC will not generate the intended code. See:
http://blog.qt.digia.com/blog/2011/06/10/type-punning-and-strict-aliasing/
A simpler solution, which doesnt need any changes to the svalue struct would have been to do the write using memcpy, which would be optimized into one pointer size store. It is defined behavior which reflects the aliasing correctly and requires only minimal modification to the original code.
Arne
On Wed, 25 Jun 2014, Chris Angelico wrote:
On Tue, Jun 24, 2014 at 7:54 PM, Chris Angelico rosuav@gmail.com wrote:
I'm starting up a completely fresh Pike clone to see if somehow I've accumulated cruft of some sort. If that behaves differently, I'll let you know.
That didn't solve anything, but the latest commit on 8.0 branch did. Thanks Grubba! Looking good.
ChrisA
Isn't also TYPE_SUBTYPE wrong in the big endian case? It seems to me that on a 64bit big endian machine both subtype and type would always end up being 0.
like so?
#define TYPE_SUBTYPE(X,Y) (((Y)|((X)<<16)) << (sizeof(ptrdiff_t) - 32))
Yes, it is currently not correct for 64-bit bit endian.
And I cannot see how a lot of code in pike would work at all if the union trick does not do so, we use union assignments in several places.
On a related note, can anyone come up with anything to put in the wasted 40 bits of struct svalue? :)
Isn't also TYPE_SUBTYPE wrong in the big endian case? It seems to me that on a 64bit big endian machine both subtype and type would always end up being 0.
Correct. Fixed last night.
like so?
#define TYPE_SUBTYPE(X,Y) (((Y)|((X)<<16)) << (sizeof(ptrdiff_t) - 32))
No, sizeof(ptrdiff_t) needs to be scaled by a factor 8...
The above version also requires casts to widen X and Y to not generate zeros anyway. Another related issue is that X and Y are often constants, and several 64-bit architectures do not have support for full 64-bit immediate constants, so I changed the field to always be 32-bit in the big-endian case.
/grubba
pike-devel@lists.lysator.liu.se
-
Arne Goedeke -
Chris Angelico -
Henrik Grubbstr�m (Lysator) ＠ Pike (-) developers forum -
Marcus Comstedt (ACROSS) (Hail Ilpalazzo!) ＠ Pike (-) developers forum
-
Per Hedbor () ＠ Pike (-) developers forum