Skip to content

Conversation

@brad0
Copy link
Contributor

@brad0 brad0 commented Nov 24, 2024

OpenBSD 7.3+ requires xonly.

OpenBSD 7.3+ requires xonly.
@brad0
Copy link
Contributor Author

brad0 commented Nov 24, 2024

cc @mstorsjo

@mstorsjo
Copy link
Contributor

FWIW, this patch breaks compilation for aarch64 for essentially every configuration concievable, as is.

With Clang:

codec/common/arm64/mc_aarch64_neon.S:35:1: error: unknown directive
.rodata
^
codec/common/arm64/mc_aarch64_neon.S:38:1: error: unknown directive
.previous
^

With binutils:

codec/common/arm64/mc_aarch64_neon.S: Assembler messages:
codec/common/arm64/mc_aarch64_neon.S:35: Error: unknown pseudo-op: `.rodata'

Switching to the .rodata section (or whatever the suitable thing on each OS is) probably needs to be wrapped up in macro in codec/common/arm64/arm_arch64_common_macro.S; based on e.g. corresponding macros in ffmpeg, it needs to be .section .rdata on Windows, .const_data for mach-o/apple and .section .rodata for the others (which I presume would work on OpenBSD as well). Then instead of .previous, we can probably either switch back to .text in an end macro for rodata sections, or add .text to WELS_ASM_AARCH64_FUNC_BEGIN.

Then finally, for MSVC build configurations, we normally use clang for building the assembly, but we used to use gas-preprocessor earlier. (Using gas-preprocessor allows using MSVC's armasm64 assembler tool, and avoids requiring two separate compilers.) In such configurations, it seems like gas-preprocessor doesn't handle cases like ldr q22, [x6, #:lo12:filter_para] - but I guess that that aspect is up to me if I want to fix gas-preprocessor to handle it, as openh264 itself doesn't normally use that configuration for MSVC builds.

@brad0
Copy link
Contributor Author

brad0 commented Nov 28, 2024

FWIW, this patch breaks compilation for aarch64 for essentially every configuration concievable, as is.

With Clang:

codec/common/arm64/mc_aarch64_neon.S:35:1: error: unknown directive
.rodata
^
codec/common/arm64/mc_aarch64_neon.S:38:1: error: unknown directive
.previous
^

Odd, OpenBSD uses Clang and we've been building this for a long time now.

With binutils:

codec/common/arm64/mc_aarch64_neon.S: Assembler messages:
codec/common/arm64/mc_aarch64_neon.S:35: Error: unknown pseudo-op: `.rodata'

Switching to the .rodata section (or whatever the suitable thing on each OS is) probably needs to be wrapped up in macro in codec/common/arm64/arm_arch64_common_macro.S; based on e.g. corresponding macros in ffmpeg, it needs to be .section .rdata on Windows, .const_data for mach-o/apple and .section .rodata for the others (which I presume would work on OpenBSD as well). Then instead of .previous, we can probably either switch back to .text in an end macro for rodata sections, or add .text to WELS_ASM_AARCH64_FUNC_BEGIN.

Then finally, for MSVC build configurations, we normally use clang for building the assembly, but we used to use gas-preprocessor earlier. (Using gas-preprocessor allows using MSVC's armasm64 assembler tool, and avoids requiring two separate compilers.) In such configurations, it seems like gas-preprocessor doesn't handle cases like ldr q22, [x6, #:lo12:filter_para] - but I guess that that aspect is up to me if I want to fix gas-preprocessor to handle it, as openh264 itself doesn't normally use that configuration for MSVC builds.

I see the macro you're referring to.

.macro  const   name, align=2, relocate=0
    .macro endconst
ELF     .size   \name, . - \name
        .purgem endconst
    .endm
#if HAVE_SECTION_DATA_REL_RO
.if \relocate
        .section        .data.rel.ro
.else
        .section        .rodata
.endif
#elif defined(_WIN32)
        .section        .rdata
#elif !defined(__MACH__)
        .section        .rodata
#else
        .const_data
#endif
        .align          \align
\name:
.endm

I didn't write any of this code, but I'll see what I can do and get back to you.

@mstorsjo
Copy link
Contributor

FWIW, this patch breaks compilation for aarch64 for essentially every configuration concievable, as is.
With Clang:

codec/common/arm64/mc_aarch64_neon.S:35:1: error: unknown directive
.rodata
^
codec/common/arm64/mc_aarch64_neon.S:38:1: error: unknown directive
.previous
^

Odd, OpenBSD uses Clang and we've been building this for a long time now.

Yeah, either this is OpenBSD specific in the Clang/LLVM assembler, or it's added in downstream patches.

I see the macro you're referring to.

.macro  const   name, align=2, relocate=0
    .macro endconst
ELF     .size   \name, . - \name
        .purgem endconst
    .endm
#if HAVE_SECTION_DATA_REL_RO
.if \relocate
        .section        .data.rel.ro
.else
        .section        .rodata
.endif
#elif defined(_WIN32)
        .section        .rdata
#elif !defined(__MACH__)
        .section        .rodata
#else
        .const_data
#endif
        .align          \align
\name:
.endm

Yep. Although this has evolved into a kinda odd and uncomfortable form - it probably can be simplified a bit, removing negations and moving the win32/apple cases above the ELF .rodata and .data.rel.ro cases (and I guess openh264 doesn't need the .data.rel.ro case); I've considered sending patches to ffmpeg and dav1d to simplify them, but haven't gotten around to that.

@mstorsjo
Copy link
Contributor

Oh, and I forgot - to get the address of the data in a different section, you probably also need to replicate something like the movrel macros in ffmpeg/dav1d, because the syntax for the adrp and add/ldr relocations differ across targets too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants