On Mon, 11 Feb 2019 11:41:08 -0800 Eric Anholt <e...@anholt.net> said:
> Carsten Haitzler <ras...@rasterman.com> writes: > > > On Mon, 04 Feb 2019 16:31:57 -0800 Eric Anholt <e...@anholt.net> said: > > > >> Carsten Haitzler <ras...@rasterman.com> writes: > >> > >> > On Fri, 1 Feb 2019 11:08:07 +0000 Emil Velikov <emil.l.veli...@gmail.com> > >> > said: > >> > > >> >> Hi Carsten, > >> >> > >> >> On 2019/01/31, Carsten Haitzler wrote: > >> >> > On Wed, 30 Jan 2019 18:33:35 +0000 Emil Velikov > >> >> > <emil.l.veli...@gmail.com> said: > >> >> > > >> >> > You might want to hold off on this. My bugfix was actually patched > >> >> > out by partly removing some of it. The void ptr math should never > >> >> > have been there and wasn't in the final patch. > >> >> > > >> >> > I'm talking about: > >> >> > > >> >> > + void *cpu2 = cpu + 8; > >> >> > > >> >> > In 300d3ae8b1445b5060f92c77c0f577f4b7b2c7d6 > >> >> > > >> >> > At least with gcc8 mesa is a dud on Raspberry Pi (can't > >> >> > upload/downlaod textures without crashing) without the fixes. I moved > >> >> > the secondary ptr math into the ASM chunk because the C compiler > >> >> > seemed to just mess up cpu2 ptr content/value for me on gcc8 (it also > >> >> > kept the parameter inputs/outputs cleaner and consistent with other > >> >> > ASM chunks). Keeping this as void ptr math alone is just wrong and > >> >> > asking for trouble and as it unfixed a fix I already had in submitted > >> >> > patches. > >> >> > > >> >> > Being at FOSDEM I now no longer have access to my OS image with all of > >> >> > this set up to test and won't until next week. I can't dig in and > >> >> > verify. Without my fixes at all it's a dead man walking with gcc8, > >> >> > and thus Arch Linux is broken entirely on Rpi without it (and has > >> >> > been for a while now). > >> > >> FWIW, my testing was done on gcc 8.30 on raspberry pi. > > > > I finally have time and am back with my Pi box. This was gcc 8.2.0 that I > > was using. > > > >> I skipped the part of moving the C expression into the asm because it > >> didn't make sense, and appeared in the series before the part that > >> actually fixed the asm clobbers bug, so it (like the .fpu neon part) > >> looked like random hacks. > > > > I did explicitly break just and only that change out. 0004 in the series was > > just that. The log explained compiler bugs prevent calculating the address > > in C (it ends up junk) so moved it to the asm block. That required changing > > the cpu2 refs all to be a register instead and add this register to the > > clobber list, so of course the patch was more than just a 2 liner, but it > > was straightforward. > > I'm quite skeptical that it's a compiler bug instead of a bug on our > end. If we have a bug in our constraints, I want to fix that bug rather > than papering over it such that we just don't tickle it on your > particular compiler/flags combination. Even if it's a compiler bug, we > should figure it out and report it. > > If you don't have the time to figure out the root cause, let's see if I > can. What compiler flags are you seeing used in your build? I've been > using piglit's texsubimage to test with various compiler flags to try to > reproduce your issue, and haven't managed to with a debug, > debugoptimized or release build in meson on Mesa master. I'm just using the standard aur build on a clean system with no customizations installed. the meson command being used that includes that .h file is: cc -Isrc/gallium/drivers/vc4/691f666@@vc4@sta -Isrc/gallium/drivers/vc4 -I../mesa/src/gallium/drivers/vc4 -Isrc -I../mesa/src -Iinclude -I../mesa/include -I../mesa/src/gallium/include -Isrc/gallium/auxiliary -I../mesa/src/gallium/auxiliary -Isrc/broadcom -I../mesa/src/broadcom -Isrc/broadcom/cle -I../mesa/src/broadcom/cle -Isrc/gallium/drivers -I../mesa/src/gallium/drivers -I../mesa/include/drm-uapi -Isrc/compiler/nir -I../mesa/src/compiler/nir -I/usr/include/libdrm -fdiagnostics-color=always -DNDEBUG -pipe -D_FILE_OFFSET_BITS=64 -std=c99 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS '-DPACKAGE_VERSION=\"19.0.0-devel\"' '-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";' -DGLX_USE_TLS -DHAVE_ST_VDPAU -DENABLE_ST_OMX_BELLAGIO=1 -DENABLE_ST_OMX_TIZONIA=0 -DHAVE_X11_PLATFORM -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DGLX_USE_DRM -DHAVE_DRM_PLATFORM -DHAVE_SURFACELESS_PLATFORM -DENABLE_SHADER_CACHE -DHAVE___BUILTIN_BSWAP32 -DHAVE___BUILTIN_BSWAP64 -DHAVE___BUILTIN_CLZ -DHAVE___BUILTIN_CLZLL -DHAVE___BUILTIN_CTZ -DHAVE___BUILTIN_EXPECT -DHAVE___BUILTIN_FFS -DHAVE___BUILTIN_FFSLL -DHAVE___BUILTIN_POPCOUNT -DHAVE___BUILTIN_POPCOUNTLL -DHAVE___BUILTIN_UNREACHABLE -DHAVE_FUNC_ATTRIBUTE_CONST -DHAVE_FUNC_ATTRIBUTE_FLATTEN -DHAVE_FUNC_ATTRIBUTE_MALLOC -DHAVE_FUNC_ATTRIBUTE_PURE -DHAVE_FUNC_ATTRIBUTE_UNUSED -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT -DHAVE_FUNC_ATTRIBUTE_WEAK -DHAVE_FUNC_ATTRIBUTE_FORMAT -DHAVE_FUNC_ATTRIBUTE_PACKED -DHAVE_FUNC_ATTRIBUTE_RETURNS_NONNULL -DHAVE_FUNC_ATTRIBUTE_VISIBILITY -DHAVE_FUNC_ATTRIBUTE_ALIAS -DHAVE_FUNC_ATTRIBUTE_NORETURN -D_GNU_SOURCE -DUSE_GCC_ATOMIC_BUILTINS -DUSE_ARM_ASM -DMAJOR_IN_SYSMACROS -DHAVE_SYS_SYSCTL_H -DHAVE_LINUX_FUTEX_H -DHAVE_ENDIAN_H -DHAVE_DLFCN_H -DHAVE_STRTOF -DHAVE_MKOSTEMP -DHAVE_POSIX_MEMALIGN -DHAVE_TIMESPEC_GET -DHAVE_MEMFD_CREATE -DHAVE_STRTOD_L -DHAVE_DLADDR -DHAVE_DL_ITERATE_PHDR -DHAVE_ZLIB -DHAVE_PTHREAD -DHAVE_PTHREAD_SETAFFINITY -DHAVE_LIBDRM -DHAVE_LLVM=0x0700 -DMESA_LLVM_VERSION_PATCH=1 -DUSE_LIBGLVND=1 -DHAVE_WAYLAND_PLATFORM -DWL_HIDE_DEPRECATED -DHAVE_DRI3 -DHAVE_DRI3_MODIFIERS -DHAVE_GALLIUM_EXTRA_HUD=1 -DHAVE_LIBSENSORS=1 -Werror=implicit-function-declaration -Werror=missing-prototypes -Werror=return-type -fno-math-errno -fno-trapping-math -Wno-missing-field-initializers -Wno-format-truncation -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16 -O2 -fstack-protector-strong -fno-plt -g -fvar-tracking-assignments -fdebug-prefix-map=/home/alarm/aur/mesa-git/src=/usr/src/debug -O1 -D_FORTIFY_SOURCE=2 -fPIC -fvisibility=hidden -MD -MQ 'src/gallium/drivers/vc4/691f666@@vc4@sta/vc4_tiling_lt.c.o' -MF 'src/gallium/drivers/vc4/691f666@@vc4@sta/vc4_tiling_lt.c.o.d' -o 'src/gallium/drivers/vc4/691f666@@vc4@sta/vc4_tiling_lt.c.o' -c ../mesa/src/gallium/drivers/vc4/vc4_tiling_lt.c that's that the arch linux PKGBUILD ends up using by default at any rate. gcc is: gcc (GCC) 8.2.0 cpu2 is even declared "+r" so the compiler can't assume the value of cpu2 post-asm block, so i don't think this here is wrong. -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- Carsten Haitzler - ras...@rasterman.com _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
