Re: [PATCH] support for the x32 architecture

Hello,

> So, I'm sorry, but I really don't know to deal with these instructions
> of yours.

"This nose of yours is false!"
OK, seriously.
We can ask gcc to create memory refs for __asm__ automatically.
In fact, we do that in a lot of MMX code. However, when processing
large contigous mem like in memcpy.c, it is more efficient to get an
address into a register once, then maintain it ourselves.
Usually, address load and pointer usage are of same size, and we
need just 1 register definition for everything. Unfortunately, x32
opens a pitfall there. Example:

   item->nums[i]++;

could be made like this:

   movl    .Litem, %edx
   movl    .Li, %ebx
   movl    88(%edx,%ebx,4), %eax
   addl    $1, %eax
   movl    %eax, 88(%edx,%ebx,4)

That is, our pointer stays 32bit %edx all the time. This _does_
work but forces 2 performance penalties into the "(...)" lines.
First, both machine codes will get a leading "prefix" byte
indicating 32bit pointer mode. This makes the code larger.
Second, decoding that prefix and 64bit extending the address
%edx -> %rdx on every use also makes the code slower.
Better solution: remember that the initial 32bit load extended
to 64bit already, and use full size from now on:

   movl    .Litem, %edx
   movl    .Li, %ebx
   movl    88(%rdx,%rbx,4), %eax
   addl    $1, %eax
   movl    %eax, 88(%rdx,%rbx,4)

Note that both pointer %rdx and index %rbx need to have same size,
mixing is not supported.
This is where my "q" hack comes in. gcc did map pointer and index
to 32bit, and I tweak them later like this:

   movl    88(%q0,%q1,4), %eax
   addl    $1, %eax
   movl    %eax, 88(%q0,%q1,4)

Of course, this hack applies _only_ to x32.

> I'm now attaching here the `make -k V=1` part of revision, hopefully it
> will show all of the errors and you can see about a proper real patch.
> 13322:34ea545d4b88 (i.e. currend tip of the default branch).

1) Thanks.
2) No such version. I guess you mean 13321:34ea545d4b88.
3) ?? confusion ??
4) Seems at least 1 of your cpp/gcc/gas has a bug.
    As soon as a "operand type mismatch for" message shows up,
    line numbers are off by at least 5. Hmm.
5) Found a possible scenario that can explain (almost?) all:
    "configure" did _not_ recognize you want x32, and set
    ARCH_X86_64 instead. "gcc" nevertheless switched to x32,
    maybe it is the default for your sys.

"configure" relies on Autoconf to create a proper host string.
If it is not x32 aware, trap hit right there. Next pit: there
might be more text after "-gnux32". Tried to fix that.
Anyway. please say
   $ < config.log grep "host"
and look for lines starting with "host=" and "host_alias=".
What do they say?
Also,
   $ < includes/configure.h grep ARCH_
reveals what configure thinks what we want.

>> Speaking of tests, It would be nice if someone could test the x32
>> _functionality_. Your initial patch did fix x32 _build_ but actually
>> did not care about (and possibly broke) some other assembly parts.

> Sure, do you have any suggestions on how to do it?  xine-lib doesn't
> seem to have any testsuite in it, I would I go on properly testing it?

A test suite for a general purpose media player? Really?
Do you (and more importantly, hg.debian.org) have lots of harddisk
space for test movies?
For now, you can try to
1) Build a sane x32 binary at all.
2) Set "memcpy method" to "kernel" or "SSE".
3) Set "audio visualzation" to "goom".
4) Play a sound file.

> FWIW, in the meantime I'll see about getting version 1.2.8 in use in
> Debian, and then will probably wait for the 1.2.9 release, as the
> commits don't apply cleanly enough (you also did a lot of changes in the
> indentation!) on top of 1.2.8)

You really need to recreate every single patch? Thank god I am using
suse and not debian...

Torsten


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot