Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] intreadwrite: Indicate potential aliasing in AV_RN/AV_WN for Clang/MSVC mode
@ 2023-08-02  7:02 Martin Storsjö
  2023-08-04 11:28 ` [FFmpeg-devel] [PATCH v2] " Martin Storsjö
  0 siblings, 1 reply; 2+ messages in thread
From: Martin Storsjö @ 2023-08-02  7:02 UTC (permalink / raw)
  To: ffmpeg-devel

While MSVC proper doesn't have a way to signal potential aliasing
corresponding to __attribute__((may_alias)) (which is what
av_alias expands to, if supported), Clang does support it.

When building with Clang in MSVC mode, __GNUC__ isn't defined but
_MSC_VER is, so Clang uses the MSVC implementation of AV_RN/AV_WN
which uses __unaligned.

Add the av_alias macro there, to indicate that the reads/writes can
alias other writes. Adding the attribute within the cast itself
doesn't seem to have any effect, it only has an effect if added in
a type definition.

Alternatively the __GNUC__ specific codepath right above could also
be used whenever __clang__ is defined; numerous __GNUC__ checks
also separately check for __clang__ to use them when building with
Clang in MSVC mode.

This fixes a couple HEVC decoder tests when built with Clang 14 or
newer in MSVC mode (with issues observed on all of x86_64, armv7
and aarch64).
---
 libavutil/intreadwrite.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavutil/intreadwrite.h b/libavutil/intreadwrite.h
index c4679fdba4..126ac3c7f8 100644
--- a/libavutil/intreadwrite.h
+++ b/libavutil/intreadwrite.h
@@ -224,8 +224,12 @@ union unaligned_16 { uint16_t l; } __attribute__((packed)) av_alias;
 
 #elif defined(_MSC_VER) && (defined(_M_ARM) || defined(_M_X64) || defined(_M_ARM64)) && AV_HAVE_FAST_UNALIGNED
 
-#   define AV_RN(s, p) (*((const __unaligned uint##s##_t*)(p)))
-#   define AV_WN(s, p, v) (*((__unaligned uint##s##_t*)(p)) = (v))
+typedef __unaligned uint64_t av_alias unaligned_64;
+typedef __unaligned uint32_t av_alias unaligned_32;
+typedef __unaligned uint16_t av_alias unaligned_16;
+
+#   define AV_RN(s, p) (*((const unaligned_##s*)(p)))
+#   define AV_WN(s, p, v) (*((unaligned_##s*)(p)) = (v))
 
 #elif AV_HAVE_FAST_UNALIGNED
 
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [FFmpeg-devel] [PATCH v2] intreadwrite: Indicate potential aliasing in AV_RN/AV_WN for Clang/MSVC mode
  2023-08-02  7:02 [FFmpeg-devel] [PATCH] intreadwrite: Indicate potential aliasing in AV_RN/AV_WN for Clang/MSVC mode Martin Storsjö
@ 2023-08-04 11:28 ` Martin Storsjö
  0 siblings, 0 replies; 2+ messages in thread
From: Martin Storsjö @ 2023-08-04 11:28 UTC (permalink / raw)
  To: ffmpeg-devel

Use the GCC specific codepath for Clang in MSVC mode too.
This matches the condition used in a number of other places.

MSVC doesn't have a way to signal potential aliasing, while GCC
(and Clang) can use __attribute__((may_alias)) for this purpose.

When building with Clang in MSVC mode, __GNUC__ isn't defined but
_MSC_VER is as Clang primarily impersonates MSVC - but even then it
does support the GCC style attributes.

The GCC specific codepath uses av_alias, which expands to
the may_alias attribute if supported. The MSVC specific codepath
doesn't use av_alias so far (as MSVC doesn't support any
corresponding attribute).

This fixes a couple HEVC decoder tests when built with Clang 14 or
newer in MSVC mode (with issues observed on all of x86_64, armv7
and aarch64).
---
v2: Changed to just use the GCC specific codepath here, as suggested
by Andreas on irc.

Will push if there's no objections.
---
 libavutil/intreadwrite.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/intreadwrite.h b/libavutil/intreadwrite.h
index c4679fdba4..21df7887f3 100644
--- a/libavutil/intreadwrite.h
+++ b/libavutil/intreadwrite.h
@@ -213,7 +213,7 @@ typedef union {
  * by per-arch headers.
  */
 
-#if defined(__GNUC__)
+#if defined(__GNUC__) || defined(__clang__)
 
 union unaligned_64 { uint64_t l; } __attribute__((packed)) av_alias;
 union unaligned_32 { uint32_t l; } __attribute__((packed)) av_alias;
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-08-04 11:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-02  7:02 [FFmpeg-devel] [PATCH] intreadwrite: Indicate potential aliasing in AV_RN/AV_WN for Clang/MSVC mode Martin Storsjö
2023-08-04 11:28 ` [FFmpeg-devel] [PATCH v2] " Martin Storsjö

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git