From 3264cf697f5fe996c2a79c6c34181c186fe2f96e Mon Sep 17 00:00:00 2001 From: Michael Panzlaff Date: Sat, 9 May 2020 14:49:51 +0200 Subject: [PATCH] fix aggressive loop optimizations Previously, aggressive loop optimizations with a new compiler were not possible due to undefined behaviour at end of arrays. A macro "UBFIX" is added to allow ifdefs for fixes which resolve undefined behavior. For example newer GCC versions will detect various bugs in the original game code and will otherwise not compile with -Werror. --- Makefile | 4 ++-- include/config.h | 8 +++++++ include/global.tv.h | 7 ++++++ src/berry_blender.c | 5 ++++ src/berry_crush.c | 5 ++++ src/easy_chat.c | 5 ++++ src/fieldmap.c | 7 ++++++ src/image_processing_effects.c | 44 +++++++++++++++++----------------- src/slot_machine.c | 23 ++++++++---------- src/tv.c | 16 ++++++------- 10 files changed, 79 insertions(+), 45 deletions(-) diff --git a/Makefile b/Makefile index 27d77754c0..53a2c7f256 100644 --- a/Makefile +++ b/Makefile @@ -67,7 +67,7 @@ OBJ_DIR := build/emerald LIBPATH := -L ../../tools/agbcc/lib else CC1 = $(shell $(CC) --print-prog-name=cc1) -quiet -override CFLAGS += -mthumb -mthumb-interwork -O2 -mabi=apcs-gnu -mtune=arm7tdmi -march=armv4t -fno-toplevel-reorder -fno-aggressive-loop-optimizations -Wno-pointer-to-int-cast +override CFLAGS += -mthumb -mthumb-interwork -O2 -mabi=apcs-gnu -mtune=arm7tdmi -march=armv4t -fno-toplevel-reorder -Wno-pointer-to-int-cast ROM := pokeemerald_modern.gba OBJ_DIR := build/modern LIBPATH := -L "$(dir $(shell $(CC) -mthumb -print-file-name=libgcc.a))" -L "$(dir $(shell $(CC) -mthumb -print-file-name=libc.a))" @@ -232,7 +232,7 @@ $(C_BUILDDIR)/record_mixing.o: CFLAGS += -ffreestanding $(C_BUILDDIR)/librfu_intr.o: CC1 := tools/agbcc/bin/agbcc_arm $(C_BUILDDIR)/librfu_intr.o: CFLAGS := -O2 -mthumb-interwork -quiet else -$(C_BUILDDIR)/librfu_intr.o: CFLAGS := -mthumb-interwork -O2 -mabi=apcs-gnu -mtune=arm7tdmi -march=armv4t -fno-toplevel-reorder -fno-aggressive-loop-optimizations -Wno-pointer-to-int-cast +$(C_BUILDDIR)/librfu_intr.o: CFLAGS := -mthumb-interwork -O2 -mabi=apcs-gnu -mtune=arm7tdmi -march=armv4t -fno-toplevel-reorder -Wno-pointer-to-int-cast endif ifeq ($(NODEP),1) diff --git a/include/config.h b/include/config.h index 318ed39d84..4f97a12a3b 100644 --- a/include/config.h +++ b/include/config.h @@ -26,4 +26,12 @@ #define UNITS_METRIC #endif +// Various undefined behavior bugs may or may not prevent compilation with +// newer compilers. So always fix them when using a modern compiler. +#if MODERN +#ifndef UBFIX +#define UBFIX +#endif +#endif + #endif // GUARD_CONFIG_H diff --git a/include/global.tv.h b/include/global.tv.h index 34791d43c9..ba1c3c37ee 100644 --- a/include/global.tv.h +++ b/include/global.tv.h @@ -18,6 +18,13 @@ typedef union // size = 0x24 /*0x23*/ u8 trainerIdHi; } common; + // Common init (used for initialization loop) + struct { + /*0x00*/ u8 kind; + /*0x01*/ bool8 active; + /*0x02*/ u8 pad02[34]; + } commonInit; + // Local shows // TVSHOW_FAN_CLUB_LETTER struct { diff --git a/src/berry_blender.c b/src/berry_blender.c index 2a122541d4..4d3b7eb6f9 100644 --- a/src/berry_blender.c +++ b/src/berry_blender.c @@ -2202,6 +2202,11 @@ static s16 sub_8081BD4(void) return sUnknown_03000E06; } +#if MODERN +// TODO remove this as soon as the code below is understood +// add a UBFIX if required (code buggy?) +__attribute__((optimize("no-aggressive-loop-optimizations"))) +#endif static void Blender_CalculatePokeblock(struct BlenderBerry *berries, struct Pokeblock *pokeblock, u8 playersNo, u8 *flavors, u16 maxRPM) { s32 i, j; diff --git a/src/berry_crush.c b/src/berry_crush.c index 48e4ac4c33..48ad7f4e3c 100755 --- a/src/berry_crush.c +++ b/src/berry_crush.c @@ -3209,6 +3209,11 @@ static u32 sub_8024568(__attribute__((unused)) struct BerryCrushGame *r0, __attr return 0; } +#if MODERN +// TODO remove this as soon as the code below is understood +// add a UBFIX if required (code buggy?) +__attribute__((optimize("no-aggressive-loop-optimizations"))) +#endif void sub_8024578(struct BerryCrushGame *r4) { u8 r5 = 0; diff --git a/src/easy_chat.c b/src/easy_chat.c index 1bdb3fdc09..2a4c49213a 100644 --- a/src/easy_chat.c +++ b/src/easy_chat.c @@ -5273,12 +5273,17 @@ void InitEasyChatPhrases(void) gSaveBlock1Ptr->mail[i].words[j] = 0xFFFF; } +#ifndef UBFIX // BUG: This is supposed to clear 64 bits, but this loop is clearing 64 bytes. // However, this bug has no resulting effect on gameplay because only the // Mauville old man data is corrupted, which is initialized directly after // this function is called when starting a new game. for (i = 0; i < 64; i++) gSaveBlock1Ptr->additionalPhrases[i] = 0; +#else + for (i = 0; i < ARRAY_COUNT(gSaveBlock1Ptr->additionalPhrases); i++) + gSaveBlock1Ptr->additionalPhrases[i] = 0; +#endif } static bool8 sub_811F28C(void) diff --git a/src/fieldmap.c b/src/fieldmap.c index e953e0f935..25157ebb16 100644 --- a/src/fieldmap.c +++ b/src/fieldmap.c @@ -533,9 +533,16 @@ static bool32 SavedMapViewIsEmpty(void) u16 i; u32 marker = 0; +#ifndef UBFIX // BUG: This loop extends past the bounds of the mapView array. Its size is only 0x100. for (i = 0; i < 0x200; i++) marker |= gSaveBlock1Ptr->mapView[i]; +#else + // UBFIX: Only iterate over 0x100 + for (i = 0; i < ARRAY_COUNT(gSaveBlock1Ptr->mapView); i++) + marker |= gSaveBlock1Ptr->mapView[i]; +#endif + if (marker == 0) return TRUE; diff --git a/src/image_processing_effects.c b/src/image_processing_effects.c index 13794faf0b..cbd8b9b374 100644 --- a/src/image_processing_effects.c +++ b/src/image_processing_effects.c @@ -5,7 +5,7 @@ // IWRAM common u8 gCanvasColumnStart; -u16 (*gCanvasPixels)[][32]; +u16 *gCanvasPixels; u8 gCanvasRowEnd; u8 gCanvasHeight; u8 gCanvasColumnEnd; @@ -125,7 +125,7 @@ static void ApplyImageEffect_RedChannelGrayscale(u8 delta) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -150,7 +150,7 @@ static void ApplyImageEffect_RedChannelGrayscaleHighlight(u8 highlight) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -179,7 +179,7 @@ static void ApplyImageEffect_Grayscale(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -195,7 +195,7 @@ static void ApplyImageEffect_Blur(void) for (i = 0; i < gCanvasColumnEnd; i++) { - u16 *pixelRow = &(*gCanvasPixels)[0][gCanvasRowStart * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[gCanvasRowStart * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart + i]; u16 prevPixel = *pixel; @@ -221,7 +221,7 @@ static void ApplyImageEffect_PersonalityColor(u8 personality) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -237,7 +237,7 @@ static void ApplyImageEffect_BlackAndWhite(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -255,7 +255,7 @@ static void ApplyImageEffect_BlackOutline(void) // Handle top row of pixels first. for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; pixel = &pixelRow[gCanvasColumnStart]; *pixel = QuantizePixel_BlackOutline(pixel, pixel + 1); for (i = 1, pixel++; i < gCanvasColumnEnd - 1; i++, pixel++) @@ -270,7 +270,7 @@ static void ApplyImageEffect_BlackOutline(void) // Handle each column from left to right. for (i = 0; i < gCanvasColumnEnd; i++) { - u16 *pixelRow = &(*gCanvasPixels)[0][gCanvasRowStart * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[gCanvasRowStart * gCanvasWidth]; pixel = &pixelRow[gCanvasColumnStart + i]; *pixel = QuantizePixel_BlackOutline(pixel, pixel + gCanvasWidth); for (j = 1, pixel += gCanvasWidth; j < gCanvasRowEnd - 1; j++, pixel += gCanvasWidth) @@ -289,7 +289,7 @@ static void ApplyImageEffect_Invert(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -306,7 +306,7 @@ static void ApplyImageEffect_Shimmer(void) u16 prevPixel; // First, invert all of the colors. - pixel = (*gCanvasPixels)[0]; + pixel = gCanvasPixels; for (i = 0; i < 64; i++) { for (j = 0; j < 64; j++, pixel++) @@ -319,7 +319,7 @@ static void ApplyImageEffect_Shimmer(void) // Blur the pixels twice. for (j = 0; j < 64; j++) { - pixel = &(*gCanvasPixels)[0][j]; + pixel = &gCanvasPixels[j]; prevPixel = *pixel; *pixel = 0x8000; for (i = 1, pixel += 64; i < 63; i++, pixel += 64) @@ -332,7 +332,7 @@ static void ApplyImageEffect_Shimmer(void) } *pixel = 0x8000; - pixel = &(*gCanvasPixels)[0][j]; + pixel = &gCanvasPixels[j]; prevPixel = *pixel; *pixel = 0x8000; for (i = 1, pixel += 64; i < 63; i++, pixel += 64) @@ -350,7 +350,7 @@ static void ApplyImageEffect_Shimmer(void) // Finally, invert colors back to the original color space. // The above blur causes the outline areas to darken, which makes // this inversion give the effect of light outlines. - pixel = (*gCanvasPixels)[0]; + pixel = gCanvasPixels; for (i = 0; i < 64; i++) { for (j = 0; j < 64; j++, pixel++) @@ -367,7 +367,7 @@ static void ApplyImageEffect_BlurRight(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; u16 prevPixel = *pixel; for (i = 1, pixel++; i < gCanvasColumnEnd - 1; i++, pixel++) @@ -387,7 +387,7 @@ static void ApplyImageEffect_BlurDown(void) for (i = 0; i < gCanvasColumnEnd; i++) { - u16 *pixelRow = &(*gCanvasPixels)[0][gCanvasRowStart * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[gCanvasRowStart * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart + i]; u16 prevPixel = *pixel; for (j = 1, pixel += gCanvasWidth; j < gCanvasRowEnd - 1; j++, pixel += gCanvasWidth) @@ -445,7 +445,7 @@ static void AddPointillismPoints(u16 arg0) for (i = 0; i < points[0].delta; i++) { - u16 *pixel = &(*gCanvasPixels)[points[i].row * 2][points[i].column]; + u16 *pixel = &gCanvasPixels[points[i].row * 64] + points[i].column; if (!(0x8000 & *pixel)) { @@ -910,7 +910,7 @@ static void QuantizePalette_Standard(bool8 useLimitedPalette) gCanvasPalette[maxIndex] = RGB2(15, 15, 15); for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -978,7 +978,7 @@ static void QuantizePalette_BlackAndWhite(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -1009,7 +1009,7 @@ static void QuantizePalette_GrayscaleSmall(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -1027,7 +1027,7 @@ static void QuantizePalette_Grayscale(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { @@ -1045,7 +1045,7 @@ static void QuantizePalette_PrimaryColors(void) for (j = 0; j < gCanvasRowEnd; j++) { - u16 *pixelRow = &(*gCanvasPixels)[0][(gCanvasRowStart + j) * gCanvasWidth]; + u16 *pixelRow = &gCanvasPixels[(gCanvasRowStart + j) * gCanvasWidth]; u16 *pixel = &pixelRow[gCanvasColumnStart]; for (i = 0; i < gCanvasColumnEnd; i++, pixel++) { diff --git a/src/slot_machine.c b/src/slot_machine.c index 9e94d11f02..d1cb05b180 100644 --- a/src/slot_machine.c +++ b/src/slot_machine.c @@ -384,7 +384,7 @@ static const u16 gSlotPayouts[]; static const u8 *const gUnknown_083EDCE4; static const u8 *const gUnknown_083EDCDC; static const u32 gReelTimeGfx[]; -static const struct SpriteSheet gSlotMachineSpriteSheets[]; +static const struct SpriteSheet gSlotMachineSpriteSheets[22]; static const struct SpritePalette gSlotMachineSpritePalettes[]; static const u16 *const gUnknown_083EDE20; static const s16 gInitialReelPositions[][2]; @@ -4171,8 +4171,8 @@ static void sub_81063C0(void) LZDecompressWram(gSlotMachineReelTime_Gfx, sUnknown_0203AAD4); sUnknown_0203AAD8 = Alloc(0x3600); LZDecompressWram(gReelTimeGfx, sUnknown_0203AAD8); - sUnknown_0203AB30 = AllocZeroed(sizeof(struct SpriteSheet) * 22); - for (i = 0; i < 22; i++) + sUnknown_0203AB30 = AllocZeroed(sizeof(struct SpriteSheet) * ARRAY_COUNT(gSlotMachineSpriteSheets)); + for (i = 0; i < ARRAY_COUNT(gSlotMachineSpriteSheets); i++) { sUnknown_0203AB30[i].data = gSlotMachineSpriteSheets[i].data; sUnknown_0203AB30[i].size = gSlotMachineSpriteSheets[i].size; @@ -6708,7 +6708,7 @@ static const struct SubspriteTable *const gUnknown_083EDBC4[] = NULL }; -static const struct SpriteSheet gSlotMachineSpriteSheets[] = +static const struct SpriteSheet gSlotMachineSpriteSheets[22] = { { .data = gSlotMachineReelSymbol1Tiles, .size = 0x200, .tag = 0 }, { .data = gSlotMachineReelSymbol2Tiles, .size = 0x200, .tag = 1 }, @@ -6727,15 +6727,12 @@ static const struct SpriteSheet gSlotMachineSpriteSheets[] = { .data = gSlotMachineNumber7Tiles, .size = 0x40, .tag = 14 }, { .data = gSlotMachineNumber8Tiles, .size = 0x40, .tag = 15 }, { .data = gSlotMachineNumber9Tiles, .size = 0x40, .tag = 16 }, -}; - -static const u8 sUnused1[][8] = -{ - {0, 0, 0, 0, 0, 2, 18}, - {0, 0, 0, 0, 0, 2, 19}, - {0, 0, 0, 0, 0, 3, 20}, - {0, 0, 0, 0, 0, 3, 21}, - {0, 0, 0, 0, 0, 0, 0} + // the data for these sheets is determined at runtime + { .data = NULL, .size = 0x200, .tag = 18 }, + { .data = NULL, .size = 0x200, .tag = 19 }, + { .data = NULL, .size = 0x300, .tag = 20 }, + { .data = NULL, .size = 0x300, .tag = 21 }, + {}, }; static const u8 *const gUnknown_083EDCDC = gUnknown_08DD19F8; diff --git a/src/tv.c b/src/tv.c index eaf30f82b8..b3722e87a2 100644 --- a/src/tv.c +++ b/src/tv.c @@ -764,11 +764,11 @@ void ClearTVShowData(void) for (i = 0; i < ARRAY_COUNT(gSaveBlock1Ptr->tvShows); i ++) { - gSaveBlock1Ptr->tvShows[i].common.kind = 0; - gSaveBlock1Ptr->tvShows[i].common.active = 0; - for (j = 0; j < sizeof(TVShow) - 2; j ++) + gSaveBlock1Ptr->tvShows[i].commonInit.kind = 0; + gSaveBlock1Ptr->tvShows[i].commonInit.active = 0; + for (j = 0; j < ARRAY_COUNT(gSaveBlock1Ptr->tvShows[i].commonInit.pad02); j ++) { - gSaveBlock1Ptr->tvShows[i].common.pad02[j] = 0; + gSaveBlock1Ptr->tvShows[i].commonInit.pad02[j] = 0; } } ClearPokemonNews(); @@ -3167,11 +3167,11 @@ void DeleteTVShowInArrayByIdx(TVShow *shows, u8 idx) { u8 i; - shows[idx].common.kind = TVSHOW_OFF_AIR; - shows[idx].common.active = FALSE; - for (i = 0; i < 34; i ++) + shows[idx].commonInit.kind = TVSHOW_OFF_AIR; + shows[idx].commonInit.active = FALSE; + for (i = 0; i < ARRAY_COUNT(shows[idx].commonInit.pad02); i++) { - shows[idx].common.pad02[i] = 0; + shows[idx].commonInit.pad02[i] = 0; } }