Bug 281179 - devel/llvm1[789] removal of arm_bf16.h from _BE_INCS_ARM in Makefile's was incorrect
Summary: devel/llvm1[789] removal of arm_bf16.h from _BE_INCS_ARM in Makefile's was in...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Brooks Davis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-09-01 06:35 UTC by Mark Millard
Modified: 2024-09-22 21:33 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (brooks)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Millard 2024-09-01 06:35:44 UTC
For devel/llvm1[789]:

clang/lib/Basic/Targets/ARM.cpp has:

 if (HasBFloat16) {
   Builder.defineMacro("__ARM_FEATURE_BF16", "1");
   Builder.defineMacro("__ARM_FEATURE_BF16_VECTOR_ARITHMETIC", "1");
   Builder.defineMacro("__ARM_BF16_FORMAT_ALTERNATIVE", "1");
 }

(The above is because target ARM in llvm covers AArch32, even for armv8.)

clang/lib/Basic/Targets/AArch64.cpp has:

 if (HasBFloat16) {
   Builder.defineMacro("__ARM_FEATURE_BF16", "1");
   Builder.defineMacro("__ARM_FEATURE_BF16_VECTOR_ARITHMETIC", "1");
   Builder.defineMacro("__ARM_BF16_FORMAT_ALTERNATIVE", "1");
 }

(The above is because target AArch64 in llvm covers only AArch64, not AARch32.)

The above suggests bf16 support has 32-bit support (even if it is armv8
32-bit). Looking for AArch32 state in:

DDI0487K_a_a-profile_architecture_reference_manual.pdf

it says (via the AArch32 column of a table):

BF16 Supported if FEAT_AA32BF16 is implemented.

So the documentation agrees about there being AARch32 support
possible. (There is also AArch64 support possible.)


Also: I've tested that putting arm_bf16.h back in the llvm18
Makefile to allow part of a poudriere-devel based armv7 www/firefox
build attempt to work instead of getting an error about an
include referencing <arm_bf16.h> not finding the file. So the
deletion lead to part of a port/package build being broken. (The
port has more broken than just that.)

For reference the tested change was:

-_BE_INCS_ARM=          arm_cde.h arm_fp16.h arm_mve.h arm_neon.h arm_sme.h \
+_BE_INCS_ARM=          arm_bf16.h arm_cde.h arm_fp16.h arm_mve.h arm_neon.h arm_sme.h \

(My Makefile has other personal differences from the official 
Makefile as well.)
Comment 1 Mark Millard 2024-09-01 07:05:28 UTC
Note: the automatic guess that lead to "freebsd-gecko (Nobody)"
being assigned is wrong: devel/llvm1[789] are not part of
gecko.
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2024-09-03 15:30:10 UTC
^Triage: correct assignment.

Fwiw, the autoassigner is not intelligent enough to know what to do with regexes.

Once upon a time I tried to write some similar code, and came to the conclusion
that simply editing PR Summaries was easier (and far less prone to false
positives).
Comment 3 Mark Linimon freebsd_committer freebsd_triage 2024-09-03 16:07:53 UTC
(In reply to Mark Linimon from comment #2)
fwiw, in case of autoassigner false positives, please feel free to email
bugmeister@FreeBSD.org directly.
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-09-03 21:40:14 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=f3785e527f7f924b78c7b9049db40faa1ba0c095

commit f3785e527f7f924b78c7b9049db40faa1ba0c095
Author:     Brooks Davis <brooks@FreeBSD.org>
AuthorDate: 2024-09-03 21:30:27 +0000
Commit:     Brooks Davis <brooks@FreeBSD.org>
CommitDate: 2024-09-03 21:38:28 +0000

    devel/llvm19: update to 19.1.0-rc4

    arm_bf16.h is installed by the ARM backend as well as AArch64 so handle
    that.

    Patch out upstream 6d7e428df611 which traded a false negative for a
    false positive when compiling compiler-rt for i386.

    Fix default options for 32-bit powerpc arches.

    PR:             281179

 devel/llvm19/Makefile                              | 18 ++++++++++--------
 devel/llvm19/distinfo                              |  6 +++---
 devel/llvm19/files/patch-revert-6d7e428df611 (new) | 17 +++++++++++++++++
 3 files changed, 30 insertions(+), 11 deletions(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-09-12 07:16:53 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=7ff7bac6370f6bb880187ec31dcd54fd046ffcd2

commit 7ff7bac6370f6bb880187ec31dcd54fd046ffcd2
Author:     Brooks Davis <brooks@FreeBSD.org>
AuthorDate: 2024-09-04 17:59:57 +0000
Commit:     Brooks Davis <brooks@FreeBSD.org>
CommitDate: 2024-09-12 07:15:27 +0000

    devel/llvm[789]: restore arm_bf16.h to ARM backend

    arm_bf16.h is generated for both armv7 and aarch64 so move it back to
    ARM.  We don't support AArch64 without ARM so we can just move it and
    not need to worry about duplicate supression (I'd previously fixed the
    issue in llvm19 in a more complicated manner so this change removes the
    duplicate supression and documents why it's not needed.)

    PR:             281179
    Reported by:    Mark Millard

 devel/llvm17/Makefile |  9 ++++++---
 devel/llvm18/Makefile | 10 ++++++----
 devel/llvm19/Makefile |  7 ++++---
 3 files changed, 16 insertions(+), 10 deletions(-)
Comment 6 Mark Millard 2024-09-22 21:33:18 UTC
See comment #5 for the commit-hook notification.