Bug 219779 - [patch] devel/llvm40: Misconfiguration on ARM targets
Summary: [patch] devel/llvm40: Misconfiguration on ARM targets
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: arm Any
: --- Affects Many People
Assignee: Brooks Davis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-04 15:57 UTC by Michal Meloun
Modified: 2017-10-13 11:42 UTC (History)
5 users (show)

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


Attachments
llvm40 patch (1.39 KB, patch)
2017-10-10 11:07 UTC, Michal Meloun
no flags Details | Diff
Updated patch (1.66 KB, patch)
2017-10-11 08:23 UTC, Michal Meloun
no flags Details | Diff
llvm 5 arm patch (1.45 KB, patch)
2017-10-12 15:38 UTC, Sylvain Garrigues
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Meloun freebsd_committer 2017-06-04 15:57:27 UTC
I found several configuration problems in all llvm ports:
1) In port's Makefile, the block marked as 
   ' # keep in sync with /usr/src/lib/clang/clang.build.mk'
   is significantly out of sync with clang.build.mk :)

2) For ARM, llvm's cmake/config.guess doesn't return proper ABI.

3) CONFIGURE_TARGET is not a passed to real llvm cmake at all. (Look to
   default triple for llvm – it doesn’t contain 'portbld')

All these issues causes that default llvm configuration is invalid (for ARM).

Fix for 1) is trivial.

Fix for 2) is also easy, but question is why we must explicitly specify ABI
for armv6-portbld-freebsd12.0 target. Dimitry, please, can you comment this? 
One can expect that pure triple 'armv6-portbld-freebsd12.0' is enough for
selecting right system's ABI (but I'm not sure).

Point 3) is more interesting. Seems that
'CMAKE_ARGS+= -DLLVM_DEFAULT_TARGET_TRIPLE=${CONFIGURE_TARGET}'
fixes default triple for normal (driver based) compilation. But it looks
like JIT compilers (an API based) doesn't uses it at all (is this bug or
feature?) and  
'CMAKE_ARGS+= -DLLVM_HOST_TRIPLE=${CONFIGURE_TARGET}'
is also necessary. At this point I'm simply not sure if this is expected
behavior – if yes, then fix is easy, but it depends on resolution of point 2).

Can you, please, help me with proper resolution of remaining questionable points?

Thanks,
Michal
Comment 1 commit-hook freebsd_committer 2017-09-19 23:45:23 UTC
A commit references this bug:

Author: brooks
Date: Tue Sep 19 23:44:36 UTC 2017
New revision: 450176
URL: https://svnweb.freebsd.org/changeset/ports/450176

Log:
  Disable LLDB builds on 10.x as they don't link.

  Attempt to correct default targets. [0]

  Fix more references to clang-format50 and use a more future proof
  patching approach. [1]

  Build and install pp-trace with the EXTRAS target.

  PR:		219779 [0], 220995 [1], 222380 [2]

Changes:
  head/devel/llvm50/Makefile
  head/devel/llvm50/files/clang-patch-tools_clang_tools_clang-format_clang-format-sublime.py
  head/devel/llvm50/files/clang-patch-tools_clang_tools_clang-format_clang-format.py
  head/devel/llvm50/files/clang-patch-tools_clang_tools_clang-format_git-clang-format
  head/devel/llvm50/pkg-plist
Comment 2 Michal Meloun freebsd_committer 2017-10-10 11:07:49 UTC
Created attachment 187046 [details]
llvm40 patch

- add support for ARMv7
- according to r450176 pass CONFIGURE_TARGET as LLVM_DEFAULT_TARGET_TRIPLE and LLVM_HOST_TRIPLE
- for ARMv[67], use correct API when building CONFIGURE_TARGET
Comment 3 Michal Meloun freebsd_committer 2017-10-11 08:23:38 UTC
Created attachment 187075 [details]
Updated patch

I accidentally posted older(pre-armv7) version of patch, sorry for this.
Comment 4 Sylvain Garrigues 2017-10-11 12:57:34 UTC
I tested mmel's updated patch successfully, can we apply it?
Comment 5 Brooks Davis freebsd_committer 2017-10-11 16:48:47 UTC
I've got a build in progress and will commit shortly.
Comment 6 commit-hook freebsd_committer 2017-10-11 17:16:32 UTC
A commit references this bug:

Author: brooks
Date: Wed Oct 11 17:15:30 UTC 2017
New revision: 451764
URL: https://svnweb.freebsd.org/changeset/ports/451764

Log:
  - add support for ARMv7
  - according to r450176 pass CONFIGURE_TARGET as LLVM_DEFAULT_TARGET_TRIPLE
    and LLVM_HOST_TRIPLE
  - for ARMv[67], use correct API when building CONFIGURE_TARGET

  PR:		219779
  Submitted by:	mmel

Changes:
  head/devel/llvm40/Makefile
Comment 7 Sylvain Garrigues 2017-10-12 15:01:37 UTC
Can you also apply similar patch to devel/llvm50 which suffers from the same issues (no support for armv7 and wrong ABI)?
Comment 8 Sylvain Garrigues 2017-10-12 15:38:00 UTC
Created attachment 187108 [details]
llvm 5 arm patch

Similar llvm 5.0 patch
Comment 9 commit-hook freebsd_committer 2017-10-12 18:17:20 UTC
A commit references this bug:

Author: brooks
Date: Thu Oct 12 18:16:51 UTC 2017
New revision: 451923
URL: https://svnweb.freebsd.org/changeset/ports/451923

Log:
  - add support for ARMv7
  - for ARMv[67], use correct API when building CONFIGURE_TARGET

  PR:		219779
  Submitted by:	Sylvain Garrigues <sylvain@sylvaingarrigues.com>

Changes:
  head/devel/llvm50/Makefile
Comment 10 Michal Meloun freebsd_committer 2017-10-13 11:42:49 UTC
Thanks for commits.