Tcl Source Code

mingw64 doesn't have SEH, and apparently amd64 arch (w64) doesn't need it.

allow_comments - 1

Andreas, never mind....

VS10 has intrinsics for Win32 as well, so I was able to test this
for Win32.

Fixed in HEAD, now TclWinCPUID should work correct on all
AMD64/i586 Windows version, both with GCC and MSVC

File Added - 396480: 3059922_3.patch

AMD inline assembler version put in HEAD.

Investigated how to do the same in Win64/MSVC,
it turns out that inline assembler is not supported
any more, but in stead intrinsics should be used.

Here is a patch (3059922_3.patch) doing this.
Andreas, does this work on MSVC/64-bit? Or
- in other words - does the platform-3.1
test pass after this patch?

Thanks, Andreas! (knowing you have a
platform available.....)

File Added - 396063: 3059922_2.patch

allow_comments - 0

When running the test suite on Tcl 8.6 (copmpiled with
mingw-w64 for AMD64), I got the following
test failure (see below). Either the test case
should be disabled for Win64, or the cpuid
should be implemented. I think that cpuid
is useful for more than only the preformance
counter, e.g. determining the number of
available cores.

Here is a patch (305992_2), which should
work with both gcc and MSVC.  I definitely works
with gcc, but I don't have a MSVC 64-bit 
environment to test that part. The 64-bit
assembler parts are basically the same as
the 32-bit versions, only the SEH part is
not needed here (as Kevin already noted)

See attached patch (305992_2.patch).

==== platform-3.1 CPU ID on Windows FAILED
==== Contents of test case:

set cpudata [testwincpuid 0]
binary format iii  [lindex $cpudata 1]  [lindex $cpudata 3]  [lindex $cpudata 2] 
    
---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: operation not available
    while executing
"testwincpuid 0"
    ("uplevel" body line 2)
    invoked from within
"uplevel 1 $script"
---- errorCode: NONE
==== platform-3.1 FAILED

The correct fix, in my arrogant opinion, is to use no SEH on 64-bitters.

There's no need to protect CPUID, because all 64-bitters have it. There's also no need to *do* CPUID, because the 64-bit BIOSes have pretty well done away with the performance counter synchronisation issues that plague the 32-bitters.

The remaining SEH calls in Tcl are all protecting against Windows operating system bugs that have been fixed for years - certainly prior to the release of any 64-bit version of Windows.

So by all means fix the build if it's broken, but don't preserve the mess of assembly language code. It's not needed on Win64.

allow_comments - 1

....and to HEAD

committed to core-8-5-branch

@dgp: yes, 3059922.patch is ok. tested with tcl8.5.9rc4

@dgp:
> If you change "movl" to "movq" on
> line 428, does it make things better?

yep, that made the trick.

there's still a linker error at the end:
Creating library file: libtcl85.atclWin32Dll.o:tclWin32Dll.c:(.text+0xc4): undefined reference to `_Tcl_Finalize'
tclWinChan.o:tclWinChan.c:(.text+0xa69): undefined reference to `_CloseHandle
'
collect2: ld returned 1 exit status
make: *** [tcl85.dll] Error 1

these functions are referenced from assembler. maybe needs a different syntax.

I believe the new attached patch 3059922.patch
gets all the logic errors corrected, without adding
new assembly.  Needs testing on all windows configurations.

File Added - 386010: 3059922.patch

kennykb comments at 1910041 suggest
the approach of the patch here is the better
way to go.

I think I misread the message.

If you change "movl" to "movq" on
line 428, does it make things better?

Why are those lines being compiled?

@dgp: compiler output:
gcc -c -O2 -fomit-frame-pointer -Wall  -I"./../generic" -DTCL_TOMMATH -DMP_PREC=4 -I"./../libtommath" -I"." -DPACKAGE_NAME=\"\" -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" -DPACKAGE_BUGREPORT=\"\" -DSTDC_HEADERS=1 -DHAVE_NO_SEH=1 -DHAVE_NO_LPFN_DECLS=1 -DHAVE_ALLOCA_GCC_INLINE=1 -DHAVE_CAST_TO_UNION=1 -DTCL_CFGVAL_ENCODING=\"cp1252\" -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DTCL_CFG_OPTIMIZED=1 -DTCL_CFG_DEBUG=1   -DBUILD_tcl "tclWin32Dll.c" -o tclWin32Dll.o
tclWin32Dll.c: In function 'TclWinCPUID':
tclWin32Dll.c:1125:28: warning: unused variable 'registration'
\Temp\cc2FmRDa.s: Assembler messages:
\Temp\cc2FmRDa.s:61: Error: Incorrect register `%rax' used with `l' suffix
make: *** [tclWin32Dll.o] Error 1

Here's my updated version of Patch 1910041.
It needs testing on the mingw/amd64 system,
which I do not have.  mescalinum?  Can you
give it a test?

File Added - 386005: kai-update.patch

I knew I saw it before: This is a duplicate
of [Patch #1910041], which contains a
better solution. See my remarks there.

I cannot test it, but I think we should
be grateful to Kai Tietz and NightStrike
for providing such a complete solution.

excuse me I didn't state explicitly what the patch was going (trying) to correct, which is a compile failure on mingw w64, because it has no SEH, and the assembler workaround is valid only for x86 (pushl, movl, are not amd64 instructions).
I think the macro HAVE_NO_SEH it's set correct.
maybe the check should be #if defined(__MINGW32__) && defined(_AMD64) && defined(HAVE_NO_SEH)

I have no understanding of SEH at all, except that I know that the acronym expands to 'structured exception handling'. I also seem to remember that Kevin talked about it at some point in some way, the exact nature of which I have forgotten (It was over a year ago at least, more likely over two years). Another person which might know it is David Gravereaux IIRC. tclguy less so actually, I believe.

nijtmans' comments sound right to me.
Is it understood why HAVE_NO_SEH goes
wrong on this platform?

@dgp While we have a 64bit windows box we are using the C compiler of the platform SDK, not gcc. The mingw we are using, is only to run bash, configure, etc., and it is a 32bit version too, just checked with depends.exe, cputype is x86, not amd64. So, no, this is not one of our configurations.

This sounds to me that HAVE_NO_SEH is ill-determined
in the configure script. It should not be necessary to
test for _WIN64 separately.

Andreas, is this configuration one of your test systems?

Does this patch cause any trouble with your test systems?

tested against tcl8.5.8 and tcl8.5.9rc4

File Added - 385692: tcl8.5.8-mingw64-fix.diff