> Date: Thu, 7 Sep 2023 22:02:12 +0200
> From: Matthieu Herrb <matth...@openbsd.org>
>
> On Thu, Sep 07, 2023 at 05:24:56PM +0200, Matthieu Herrb wrote:
> > On Thu, Sep 07, 2023 at 07:11:40AM +0200, Anton Lindqvist wrote:
> > > On Wed, Sep 06, 2023 at 05:42:37AM -0600, Robert Nagy wrote:
> > > > CVSROOT: /cvs
> > > > Module name: xenocara
> > > > Changes by: rob...@cvs.openbsd.org 2023/09/06 05:42:37
> > > >
> > > > Modified files:
> > > > driver/xf86-video-amdgpu/src: amdgpu_present.c
> > > > drmmode_display.h
> > > > xserver/glamor : glamor.h glamor_egl.c
> > > >
> > > > Log message:
> > > > unbreak build with clang-16 by fixing up function definitions to match
> > > >
> > > > our uint64_t is an unsinged long long, but CARD64 is defined as
> > > > unsigned long
> > > > so the function pointer types in both glamor and xf86-video-amdgpu were
> > > > mismatched and clang-16 treats that as an error
> > > >
> > > > ok matthieu@
> > >
> > > This broke the tree. Here's a potential fix.
> >
> > Hmm no, this one reverts parts of the llvm 16 diffs.
> >
> > What about this that gets rid of CARD64 completely in this context ?
> >
> > hint for the X developpers: CARD64 and friends are normally reserved
> > for the X protocol specification and implementation
> >
> > All other uses as cheap substites for uint64_t or similar are just
> > historical artefacts from an era where there was no standard integer
> > types with known fixed lengths.
>
> This is still not enough.
>
> I've deciced to cure the problem at its root.
>
> Whith this patch, the tree builds with both base llvm and llvm 16 on
> amd64. I've started a build i386 to double check 32 bit arches.
>
> And it will allow to revert some other patches to reduce the number of
> local changes. I also think that it has some chances to be accepted
> upstreams.
>
> basically just define the CARDnn types in terms on uint_nn everywhere.
> Like for signal.h all systems still supported by X have stdint and the
> uintnn_t types.
The problem with this is that you're changing the type of CARD32 and
CARD64. Which means that if these types are used in C++ code, the
mangling changes, and therefore you risk breaking the ABI. And I bet
you there is C++ code that uses the CARD32 and CARD64 types.
How did the commit break the tree? The actual error messages were
never mailed out as far as I can tell.
I think you should simply revert to the pre-hackathon state.
> Index: proto/xorgproto/include/X11/Xmd.h
> ===================================================================
> RCS file: /local/cvs/xenocara/proto/xorgproto/include/X11/Xmd.h,v
> retrieving revision 1.2
> diff -u -p -u -r1.2 Xmd.h
> --- proto/xorgproto/include/X11/Xmd.h 11 Nov 2021 08:55:42 -0000 1.2
> +++ proto/xorgproto/include/X11/Xmd.h 7 Sep 2023 16:20:01 -0000
> @@ -57,6 +57,8 @@ SOFTWARE.
> # include <sys/isa_defs.h> /* Solaris: defines _LP64 if necessary */
> # endif
>
> +#include <stdint.h>
> +
> #if defined(__SIZEOF_LONG__)
> # if __SIZEOF_LONG__ == 8
> # define LONG64 /* 32/64-bit architecture */
> @@ -107,15 +109,10 @@ typedef short INT16;
>
> typedef signed char INT8;
>
> -# ifdef LONG64
> -typedef unsigned long CARD64;
> -typedef unsigned int CARD32;
> -# else
> -typedef unsigned long long CARD64;
> -typedef unsigned long CARD32;
> -# endif
> -typedef unsigned short CARD16;
> -typedef unsigned char CARD8;
> +typedef uint64_t CARD64;
> +typedef uint32_t CARD32;
> +typedef uint16_t CARD16;
> +typedef uint8_t CARD8;
>
> typedef CARD32 BITS32;
> typedef CARD16 BITS16;
>
> >
> > Index: src/drmmode_display.c
> > ===================================================================
> > RCS file:
> > /local/cvs/xenocara/driver/xf86-video-amdgpu/src/drmmode_display.c,v
> > retrieving revision 1.4
> > diff -u -p -u -r1.4 drmmode_display.c
> > --- src/drmmode_display.c 5 Dec 2022 16:41:17 -0000 1.4
> > +++ src/drmmode_display.c 7 Sep 2023 15:20:36 -0000
> > @@ -197,7 +197,7 @@ drmmode_wait_vblank(xf86CrtcPtr crtc, dr
> > * version and DRM kernel module configuration, the vblank
> > * timestamp can either be in real time or monotonic time
> > */
> > -int drmmode_get_current_ust(int drm_fd, CARD64 * ust)
> > +int drmmode_get_current_ust(int drm_fd, uint64_t * ust)
> > {
> > uint64_t cap_value;
> > int ret;
> > @@ -211,14 +211,14 @@ int drmmode_get_current_ust(int drm_fd,
> > ret = clock_gettime(CLOCK_MONOTONIC, &now);
> > if (ret)
> > return ret;
> > - *ust = ((CARD64) now.tv_sec * 1000000) + ((CARD64) now.tv_nsec / 1000);
> > + *ust = ((uint64_t) now.tv_sec * 1000000) + ((uint64_t) now.tv_nsec /
> > 1000);
> > return 0;
> > }
> >
> > /*
> > * Get current frame count and frame count timestamp of the crtc.
> > */
> > -int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
> > +int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, uint64_t *ust, uint64_t
> > *msc)
> > {
> > ScrnInfoPtr scrn = crtc->scrn;
> > uint32_t seq;
> > @@ -303,7 +303,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, i
> > drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> > ScrnInfoPtr scrn = crtc->scrn;
> > AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
> > - CARD64 ust;
> > + uint64_t ust;
> > int ret;
> >
> > if (drmmode_crtc->dpms_mode == DPMSModeOn && mode != DPMSModeOn) {
> > @@ -321,7 +321,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, i
> > "%s cannot get last vblank counter\n",
> > __func__);
> > else {
> > - CARD64 nominal_frame_rate, pix_in_frame;
> > + uint64_t nominal_frame_rate, pix_in_frame;
> >
> > drmmode_crtc->dpms_last_ust = ust;
> > drmmode_crtc->dpms_last_seq = seq;
> > @@ -347,7 +347,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, i
> > xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> > "%s cannot get current time\n", __func__);
> > else if (drmmode_crtc->dpms_last_ust) {
> > - CARD64 time_elapsed, delta_seq;
> > + uint64_t time_elapsed, delta_seq;
> > time_elapsed = ust - drmmode_crtc->dpms_last_ust;
> > delta_seq = time_elapsed * drmmode_crtc->dpms_last_fps;
> > delta_seq /= 1000000;
> > Index: src/drmmode_display.h
> > ===================================================================
> > RCS file:
> > /local/cvs/xenocara/driver/xf86-video-amdgpu/src/drmmode_display.h,v
> > retrieving revision 1.5
> > diff -u -p -u -r1.5 drmmode_display.h
> > --- src/drmmode_display.h 6 Sep 2023 11:42:37 -0000 1.5
> > +++ src/drmmode_display.h 7 Sep 2023 15:20:36 -0000
> > @@ -127,7 +127,7 @@ typedef struct {
> > PixmapPtr prime_scanout_pixmap;
> >
> > int dpms_mode;
> > - CARD64 dpms_last_ust;
> > + uint64_t dpms_last_ust;
> > uint32_t dpms_last_seq;
> > int dpms_last_fps;
> > uint32_t interpolated_vblanks;
> > @@ -285,7 +285,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn
> > enum drmmode_flip_sync flip_sync,
> > uint32_t target_msc);
> > int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, uint64_t *ust, uint64_t
> > *msc);
> > -int drmmode_get_current_ust(int drm_fd, CARD64 * ust);
> > +int drmmode_get_current_ust(int drm_fd, uint64_t * ust);
> > void drmmode_crtc_set_vrr(xf86CrtcPtr crtc, Bool enabled);
> >
> > Bool drmmode_wait_vblank(xf86CrtcPtr crtc, drmVBlankSeqType type,
> >
> > --
> > Matthieu Herrb
>
> --
> Matthieu Herrb
>
>
