This is an archive of the discontinued LLVM Phabricator instance.

[cc1as] Add support for emitting the build version load command for -darwin-target-variant
ClosedPublic

Authored by bc-lee on Mar 16 2022, 4:14 PM.

Details

Summary

This patch extends cc1as to export the build version load command with
LC_VERSION_MIN_MACOSX.
This is especially important for Mac Catalyst as Mac Catalyst uses
the MacOS's compiler rt built-ins.

Diff Detail

Event Timeline

bc-lee created this revision.Mar 16 2022, 4:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 4:14 PM
bc-lee requested review of this revision.Mar 16 2022, 4:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 4:14 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This may need a test in clang/test/Misc/cc1as-*.

Can you add to the description what the clang -cc1as usage looks like?

clang/tools/driver/cc1as_main.cpp
217
417
bc-lee updated this revision to Diff 417595.Mar 23 2022, 6:50 AM

Addressing review comments.

bc-lee marked 2 inline comments as done.Mar 23 2022, 6:51 AM

Hi, could you have a look at it once more?

bc-lee added a subscriber: thakis.Apr 19 2022, 8:29 PM
MaskRay accepted this revision.Apr 19 2022, 8:38 PM
MaskRay added inline comments.
clang/lib/Driver/ToolChains/Darwin.h
495

llvm::None

clang/test/Misc/cc1as-darwin-target-variant-triple.s
13

Prefer -NEXT

This revision is now accepted and ready to land.Apr 19 2022, 8:38 PM
bc-lee added inline comments.Apr 19 2022, 11:37 PM
clang/lib/Driver/ToolChains/Darwin.h
495

Do you mean to modify the method in clang/include/clang/Driver/ToolChain.h?

MaskRay added inline comments.Apr 19 2022, 11:52 PM
clang/lib/Driver/ToolChains/Darwin.h
495

Ah, yes, return llvm::Optional<llvm::Triple>(); Sorry for commenting on the wrong place.

clang/test/Misc/cc1as-darwin-target-variant-triple.s
3

-filetype obj on the %clang -cc1as line seems weird.

bc-lee updated this revision to Diff 423878.Apr 20 2022, 5:32 AM

Address review comments

bc-lee marked 3 inline comments as done.Apr 20 2022, 5:33 AM
bc-lee added inline comments.
clang/test/Misc/cc1as-darwin-target-variant-triple.s
3

Without this, I cannot create object file and validate it using llvm-readobj.

thakis added a comment.EditedApr 20 2022, 8:15 AM

Hm, is adding this flag the right thing to do? Looking at clang's -S output for (say) -target arm64-apple-macos -darwin-target-variant arm64-apple-ios13.1-macabi, it emits lines like:

	.build_version macos, 12, 0	sdk_version 12, 3
	.build_version macCatalyst, 14, 0	sdk_version 15, 4

That suggests that this should be part of the .s file instead of a flag to cc1as (?)

As you confirmed at https://crbug.com/1259122#c21, there is a problem with assembly files like floatundidf.S and this patch is intended to fix it. C/C++ files are fine.

Xcode's clang seems to support this:

 % clang -target arm64-apple-macos -target-variant arm64-apple-ios13.1-macabi foo.s -c                                  
% otool -l foo.o | rg -A2 BUILD                                                                                        
      cmd LC_BUILD_VERSION
  cmdsize 24
 platform 1
--
      cmd LC_BUILD_VERSION
  cmdsize 24
 platform 6

But normal clang doesn't:

% out/gn/bin/clang -target arm64-apple-macos -darwin-target-variant arm64-apple-ios13.1-macabi foo.s -isysroot $(xcrun -show-sdk-path) -c
% otool -l foo.o | rg -A2 BUILD                                                                                                                  
      cmd LC_BUILD_VERSION
  cmdsize 24
 platform 1

So based on that, I agree it's good to land this.

You have commit permissions, right?

clang/test/Misc/cc1as-darwin-target-variant-triple.s
3

If you use emit-obj I think this test needs a REQUIRES: x86-registered-target.

I'm not sure if piping binary data works on Windows – it might have stdout in text mode, which will do some byte replacements.

bc-lee updated this revision to Diff 424209.Apr 21 2022, 8:20 AM

Add REQUIRES: x86-registered-target on the test.

clang/test/Misc/cc1as-darwin-target-variant-triple.s
3

A search for -o -.*\n.*RUN.*llvm-readobj in the LLVM codebase suggests that there are already several tests using that pattern. Does that really matter?

Thanks!

clang/test/Misc/cc1as-darwin-target-variant-triple.s
3

Well, if it works, it doesn't matter :)

I'll land this and the bots can tell us if it works.

keith added a subscriber: keith.Aug 25 2022, 10:52 AM