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

Offline radius

  • Newbies
  • *
  • Posts: 2
  • Karma: +0/-0
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #870 on: December 20, 2017, 10:28:11 pm »
I'm not sure it is related to the bump to .40 release, as i mentioned in https://github.com/libretro/fbalpha/issues/166, it is more likely an issue with changing int to intptr_t in m68kcpu.c's m68k_context_size_no_pointers function in https://github.com/libretro/fbalpha/commit/0d21221fe9030ca8c482ca9042f92babf0a52dde .
From my understanding of "intptr_t", it doesn't return the same in 32/64 bits, and i kinda remember there was a warning related to this function and dink explicitly told me not to mess with it if i didn't want to break savestates, but someone else did and i overlooked this.

Edit : it would also be worth checking what was done on http://neo-source.com/index.php?topic=3162.0 , jan's special version mention being fully cross-platform for savestates. I don't think i'll have time to deal with bugs this week (wait for next week, i took leave), so please be patient, or try reverting the 2 changes i mentioned in https://github.com/libretro/fbalpha/issues/166 (the one with intptr_t and the one with ACB_FULLSCAN) and tell me if it fixes this issue.

It's not the fullscan thing (that one helped me with other issues like the coin counter not being saved properly in neogeo games) and I did that waaaay later.
I tried reverting that change and it didn't help though.

It's definitely the bump to .40 but that is one HUGE COMMIT so it will be difficult to narrow it down.

Offline barbudreadmon

  • Expert
  • *****
  • Posts: 273
  • Karma: +7/-0
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #871 on: December 21, 2017, 05:40:38 am »
Ok, i think this is my fault, when i told dink about the changes for padding the savestates, it seems i overlooked some of them, so they weren't included in .40 release, here is the patch for the things i overlooked : https://github.com/libretro/fbalpha/commit/9722254c6b3789b0936e883d5b63a27a7c20b81a.patch
I also reverted intptr_t to int in m68kcpu.c .

There is just one last thing in sh2.cpp, in libretro-fbalpha .39 we did this for savestates cross-platform compatibility : https://pastebin.com/0cmkDAAi
But the current code is slightly different :
Code: [Select]
ScanVar(& ( Sh2Ext[i].sh2 ), sizeof(SH2) - 4, szText);was replaced by
Code: [Select]
ScanVar(& ( Sh2Ext[i].sh2 ), sizeof(SH2), szText);
Sh2Ext[i].sh2.irq_callback = irq_callback;"
So i'm not sure how to apply the changes from https://pastebin.com/0cmkDAAi to the current version (honestly i don't even know why they were necessary for savestates cross-platform compatibility). Any suggestion would be welcome.

Offline dink

  • Administrator
  • *****
  • Posts: 2570
  • Karma: +220/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #872 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;


Offline barbudreadmon

  • Expert
  • *****
  • Posts: 273
  • Karma: +7/-0
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #873 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: 2570
  • Karma: +220/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #874 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

Offline barbudreadmon

  • Expert
  • *****
  • Posts: 273
  • Karma: +7/-0
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #875 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.

Offline barbudreadmon

  • Expert
  • *****
  • Posts: 273
  • Karma: +7/-0
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #876 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 #877 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: 2570
  • Karma: +220/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #878 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 »

Offline barbudreadmon

  • Expert
  • *****
  • Posts: 273
  • Karma: +7/-0
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #879 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 #880 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

Offline barbudreadmon

  • Expert
  • *****
  • Posts: 273
  • Karma: +7/-0
  • lr-fbalpha developer
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #881 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 #882 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: 2570
  • Karma: +220/-0
  • feed the horse yumyum
Re: FB Alpha 0.2.97.42 Bug Reports
« Reply #883 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 #884 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 »