From: Boris Brezillon <boris.brezillon@collabora.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch,
trix@redhat.com, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
naresh.kamboju@linaro.org, dakr@redhat.com
Subject: Re: [PATCH 1/2] drm/exec: use unique instead of local label
Date: Wed, 2 Aug 2023 10:44:15 +0200 [thread overview]
Message-ID: <20230802104415.7fe64b67@collabora.com> (raw)
In-Reply-To: <CAKwvOdmRm=vOjNXGWeuKw-h78CXMrkcwc5vnCRVqFxMxWzhCcg@mail.gmail.com>
On Tue, 1 Aug 2023 13:35:13 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Mon, Jul 31, 2023 at 5:36 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
> >
> > GCC forbids to jump to labels in loop conditions and a new clang
> > check stumbled over this.
> >
> > So instead using a local label inside the loop condition use an
> > unique label outside of it.
> >
> > Fixes: commit 09593216bff1 ("drm: execution context for GEM buffers v7")
> > Link: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1890
> > Link: https://github.com/llvm/llvm-project/commit/20219106060208f0c2f5d096eb3aed7b712f5067
> > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> > CC: Boris Brezillon <boris.brezillon@collabora.com>
> > Signed-off-by: Christian König <christian.koenig@amd.com>
>
> Works for me; thanks for the patch!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> I suspect it's possible to change the indirect goto into a direct goto
> with some further refactoring (macros can take block statements; if
> drm_exec_until_all_locked accepted a block statement arg then you
> could introduce a new scope, and a new local label to that scope, then
> just use direct goto),
Maybe I'm wrong, but this sounds like the version I proposed here [1].
> but this will probably apply cleaner. (oh, is
> 09593216bff1 only in next at the moment? The AuthorDate threw me.)
>
> There are some curious cases where __attribute__((cleanup())) doesn't
> mesh well with indirect gotos.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37722
>
> May not ever be a problem here...
[1]https://patchwork.freedesktop.org/patch/543077/
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: tzimmermann@suse.de,
"Christian König" <ckoenig.leichtzumerken@gmail.com>,
naresh.kamboju@linaro.org, llvm@lists.linux.dev,
linux-kernel@vger.kernel.org, mripard@kernel.org,
dakr@redhat.com, dri-devel@lists.freedesktop.org,
trix@redhat.com
Subject: Re: [PATCH 1/2] drm/exec: use unique instead of local label
Date: Wed, 2 Aug 2023 10:44:15 +0200 [thread overview]
Message-ID: <20230802104415.7fe64b67@collabora.com> (raw)
In-Reply-To: <CAKwvOdmRm=vOjNXGWeuKw-h78CXMrkcwc5vnCRVqFxMxWzhCcg@mail.gmail.com>
On Tue, 1 Aug 2023 13:35:13 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Mon, Jul 31, 2023 at 5:36 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
> >
> > GCC forbids to jump to labels in loop conditions and a new clang
> > check stumbled over this.
> >
> > So instead using a local label inside the loop condition use an
> > unique label outside of it.
> >
> > Fixes: commit 09593216bff1 ("drm: execution context for GEM buffers v7")
> > Link: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1890
> > Link: https://github.com/llvm/llvm-project/commit/20219106060208f0c2f5d096eb3aed7b712f5067
> > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> > CC: Boris Brezillon <boris.brezillon@collabora.com>
> > Signed-off-by: Christian König <christian.koenig@amd.com>
>
> Works for me; thanks for the patch!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> I suspect it's possible to change the indirect goto into a direct goto
> with some further refactoring (macros can take block statements; if
> drm_exec_until_all_locked accepted a block statement arg then you
> could introduce a new scope, and a new local label to that scope, then
> just use direct goto),
Maybe I'm wrong, but this sounds like the version I proposed here [1].
> but this will probably apply cleaner. (oh, is
> 09593216bff1 only in next at the moment? The AuthorDate threw me.)
>
> There are some curious cases where __attribute__((cleanup())) doesn't
> mesh well with indirect gotos.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37722
>
> May not ever be a problem here...
[1]https://patchwork.freedesktop.org/patch/543077/
next prev parent reply other threads:[~2023-08-02 8:44 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-31 12:36 [PATCH 1/2] drm/exec: use unique instead of local label Christian König
2023-07-31 12:36 ` [PATCH 2/2] drm/exec: add test case for using a drm_exec multiple times Christian König
2023-07-31 15:31 ` [PATCH 1/2] drm/exec: use unique instead of local label Nathan Chancellor
2023-07-31 15:31 ` Nathan Chancellor
2023-07-31 16:00 ` Boris Brezillon
2023-07-31 16:00 ` Boris Brezillon
2023-08-01 20:35 ` Nick Desaulniers
2023-08-01 20:35 ` Nick Desaulniers
2023-08-02 8:44 ` Boris Brezillon [this message]
2023-08-02 8:44 ` Boris Brezillon
2023-08-02 21:24 ` Nick Desaulniers
2023-08-02 21:24 ` Nick Desaulniers
2023-08-10 8:29 ` Rasmus Villemoes
2023-08-10 8:29 ` Rasmus Villemoes
2023-08-09 15:37 ` Nathan Chancellor
2023-08-09 15:37 ` Nathan Chancellor
2023-08-10 6:40 ` Boris Brezillon
2023-08-10 6:40 ` Boris Brezillon
2023-08-10 6:48 ` Christian König
2023-08-10 6:48 ` Christian König
2023-08-10 14:38 ` Nathan Chancellor
2023-08-10 14:38 ` Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230802104415.7fe64b67@collabora.com \
--to=boris.brezillon@collabora.com \
--cc=airlied@gmail.com \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=dakr@redhat.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=naresh.kamboju@linaro.org \
--cc=ndesaulniers@google.com \
--cc=trix@redhat.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.