Author Topic: FB Alpha 0.2.97.43 Bug Reports  (Read 261648 times)

Offline dink

  • Administrator
  • *****
  • Posts: 2889
  • Karma: +245/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #870 on: December 21, 2017, 08:31:13 AM »
barbudreadmon,
sh2 was modified so that it can scan the entire struct while preserving the pointer, so the new changes are ok.

I don't understand the need to pad this in capcom/qs_c.cpp:
Code: [Select]
- INT32 nPlayStart; // Start of being played
+ ALIGN_VAR(8) INT32 nPlayStart; // Start of being played

or this in snd/ay8910.c:
Code: [Select]
- INT32 register_latch;
+ ALIGN_VAR(8) INT32 register_latch;


Online barbudreadmon

  • Administrator
  • *****
  • Posts: 369
  • Karma: +16/-1
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #871 on: December 21, 2017, 09:03:19 AM »
barbudreadmon,
sh2 was modified so that it can scan the entire struct while preserving the pointer, so the new changes are ok.

I don't understand the need to pad this in capcom/qs_c.cpp:
Code: [Select]
- INT32 nPlayStart; // Start of being played
+ ALIGN_VAR(8) INT32 nPlayStart; // Start of being played

or this in snd/ay8910.c:
Code: [Select]
- INT32 register_latch;
+ ALIGN_VAR(8) INT32 register_latch;
I'm not sure this is the issue, but those are the only things radius and aliaspider did in libretro-fba .39 that disappeared when i rebased the code against the fba .40 release (which seems to be the first commit where the savestates compatibility issue reappeared). I'll let you know if it fixes their issue as soon as they test it, or i'll try testing this myself (my raspberry is 32 bits and my desktop is 64 bits, i should be able to test this).
« Last Edit: December 21, 2017, 09:05:34 AM by barbudreadmon »

Offline dink

  • Administrator
  • *****
  • Posts: 2889
  • Karma: +245/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #872 on: December 21, 2017, 09:42:49 AM »
I can't really recommend those 2 changes, though, because an int32 doesn't need to be padded on any system.

best regards,
- dink

Online barbudreadmon

  • Administrator
  • *****
  • Posts: 369
  • Karma: +16/-1
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #873 on: December 21, 2017, 10:26:52 AM »
I can't really recommend those 2 changes, though, because an int32 doesn't need to be padded on any system.

best regards,
- dink
I'm not so sure, here is a quote from an article :
Quote
GNU g++ has a language extension that allows you to query a type?s alignment. The following program prints fundamental type sizes and alignment requirements of a platform for which it was compiled:

#include <iostream>
 
using namespace std;
 
template <typename T>
void print (char const* name)
{
  cerr << name
       << " sizeof = " << sizeof (T)
       << " alignof = " << __alignof__ (T)
       << endl;
}
 
int main ()
{
  print<bool>        ("bool          ");
  print<wchar_t>     ("wchar_t       ");
  print<short>       ("short int     ");
  print<int>         ("int           ");
  print<long>        ("long int      ");
  print<long long>   ("long long int ");
  print<float>       ("float         ");
  print<double>      ("double        ");
  print<long double> ("long double   ");
  print<void*>       ("void*         ");
}
The following listing shows the result of running this program on a 32-bit x86 GNU/Linux machine. Notice the size and alignment of the long long, double, and long double types.

bool           sizeof = 1  alignof = 1
wchar_t        sizeof = 4  alignof = 4
short int      sizeof = 2  alignof = 2
int            sizeof = 4  alignof = 4
long int       sizeof = 4  alignof = 4
long long int  sizeof = 8  alignof = 4
float          sizeof = 4  alignof = 4
double         sizeof = 8  alignof = 4
long double    sizeof = 12 alignof = 4
void*          sizeof = 4  alignof = 4
[Actually, the above program shows that the alignment of long long and double is 8. This is, however, not the case since the IA32 ABI specifies that their alignments should be 4. Also, if you wrap long long or double in a struct and take the alignment of the resulting type, it will be 4, not 8.]

And the following listing is for 64-bit x86-64 GNU/Linux:

bool           sizeof = 1  alignof = 1
wchar_t        sizeof = 4  alignof = 4
short int      sizeof = 2  alignof = 2
int            sizeof = 4  alignof = 4
long int       sizeof = 8  alignof = 8
long long int  sizeof = 8  alignof = 8
float          sizeof = 4  alignof = 4
double         sizeof = 8  alignof = 8
long double    sizeof = 16 alignof = 16
void*          sizeof = 8  alignof = 8
source : https://www.codesynthesis.com/~boris/blog/2009/04/06/cxx-data-alignment-portability/

If i understand well (i'm still a noob at c/c++), the INT32 is a "long int", right ? This article is saying it has an alignment of 4 in 32 bits and 8 in 64 bits, the author could be saying bullshit though.

Online barbudreadmon

  • Administrator
  • *****
  • Posts: 369
  • Karma: +16/-1
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #874 on: December 21, 2017, 12:27:48 PM »
Must be it, the issue with neogeo/cps was fixed. I just need to fix cps3 now.

Offline jan_klaassen

  • FBA Dev
  • ******
  • Posts: 315
  • Karma: +10/-0
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #875 on: December 21, 2017, 01:27:32 PM »
Sizes may vary (normally only long int these days I think). You should use int32_t etc. when your int needs to be a specific width. To check at runtime what you have, use std::numeric_limits.

Offline dink

  • Administrator
  • *****
  • Posts: 2889
  • Karma: +245/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #876 on: December 21, 2017, 06:06:33 PM »
If i understand well (i'm still a noob at c/c++), the INT32 is a "long int", right ? This article is saying it has an alignment of 4 in 32 bits and 8 in 64 bits, the author could be saying bullshit though.

INT32 is a standard 32bit/4 byte integer (on all platforms), this is why the alignment to 8 bytes doesn't make much sense.

best regards,
- dink
« Last Edit: December 21, 2017, 06:11:13 PM by dink »

Online barbudreadmon

  • Administrator
  • *****
  • Posts: 369
  • Karma: +16/-1
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #877 on: December 22, 2017, 01:47:33 AM »
INT32 is a standard 32bit/4 byte integer (on all platforms), this is why the alignment to 8 bytes doesn't make much sense.

best regards,
- dink
Well, if i understand what "sizeof" do, it definitely seems INT32 is a 8 byte integer on 64 bits.

Offline jan_klaassen

  • FBA Dev
  • ******
  • Posts: 315
  • Karma: +10/-0
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #878 on: December 22, 2017, 03:34:27 AM »
long is 64 bits on some systems, 32bits on others. gcc isn't lying to you. See http://en.cppreference.com/w/cpp/language/types

Online barbudreadmon

  • Administrator
  • *****
  • Posts: 369
  • Karma: +16/-1
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #879 on: December 22, 2017, 05:09:00 AM »
long is 64 bits on some systems, 32bits on others. gcc isn't lying to you. See http://en.cppreference.com/w/cpp/language/types
Yes, actually i also heard different compilers could produce different size/alignment on the same arch.

Offline jan_klaassen

  • FBA Dev
  • ******
  • Posts: 315
  • Karma: +10/-0
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #880 on: December 22, 2017, 06:37:28 AM »
In theory, absolutely. I don't know if it's the case in practice. One thing to watch is Windows' BOOL type (which is really an int) vs C/C++ bool.

Offline dink

  • Administrator
  • *****
  • Posts: 2889
  • Karma: +245/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #881 on: December 22, 2017, 07:51:06 AM »
barbudreadmon, oh my... INT32 isn't a long!  you have your define for this type set wrongly, or perhaps, need to define it differently on those platforms.  it should be an unsigned int / 32bit / 4 bytes :)  Obviously, the 32 after INT means something,  right? :)
read here:
http://en.cppreference.com/w/cpp/language/types (thanks jan for the link)

burn.h:
typedef unsigned int                  UINT32;
typedef signed int                     INT32;

okidoki?

bye for now,
- dink
« Last Edit: December 22, 2017, 08:50:39 AM by dink »

Offline nee

  • Newbies
  • *
  • Posts: 2
  • Karma: +0/-0
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #882 on: December 22, 2017, 09:38:42 AM »
I found some broken dip switches:

galaga - setting it to freeplay breaks the game and it no longer loads past "ROM OK"
toaplan GP9001 hardware, toaplan-developed games (batsugun, batsugunsp, dogyuun, truxton2, possibly others, but not Raizing games on the same hardware) - region cannot be changed from the default (Korea for Batsugun, Japan for others), changes have no effect.

Setting these dips seems to work ok in MAME. :S
« Last Edit: December 22, 2017, 09:40:50 AM by nee »

Online barbudreadmon

  • Administrator
  • *****
  • Posts: 369
  • Karma: +16/-1
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #883 on: December 22, 2017, 11:28:41 AM »
barbudreadmon, oh my... INT32 isn't a long!  you have your define for this type set wrongly, or perhaps, need to define it differently on those platforms.  it should be an unsigned int / 32bit / 4 bytes :)  Obviously, the 32 after INT means something,  right? :)
read here:
http://en.cppreference.com/w/cpp/language/types (thanks jan for the link)

burn.h:
typedef unsigned int                  UINT32;
typedef signed int                     INT32;

okidoki?

bye for now,
- dink
You are right, i assumed int32 was a long but it's a short.
Well, hopefully i'm using the vanilla defines, but this is a fact that adding the alignment macro for those 2 int32 ended up solving the cross platform savestates issue for neogeo/cps, so there definitely seems to be something different with "signed short" on 32/64 bits. Since this is really puzzling, and could be an important information for writing cross platform code in the future, i'll write a little program to test size/alignment of those on both 32 and 64 bits system.

Edit : i confirm size/alignment of INT32 don't differ between 32 and 64 bits system (at least on gcc linux), i overlooked this one but there is also an ALIGN_VAR(8) for the void in ay8910.h, so it is probably this one which solved the issue for those drivers.
« Last Edit: December 22, 2017, 11:44:05 AM by barbudreadmon »

Offline jan_klaassen

  • FBA Dev
  • ******
  • Posts: 315
  • Karma: +10/-0
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #884 on: December 22, 2017, 12:22:35 PM »
Quote
i assumed  int32 was a long but it's a short.
I hope not -- that would make it 16 bits. Again, check using std::numeric_limits if you're not sure.