peter.smith (Peter Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 11 2016, 8:56 AM (127 w, 4 d)

Recent Activity

Yesterday

peter.smith added a comment to D51854: [Arm builtins] Remove non-necessary IS check.

Ping! I do not mind updating the patch to use 1 instruction instead of 2, as Peter said, if anyone prefers that.

Thu, Sep 20, 8:45 AM

Wed, Sep 19

peter.smith accepted D50297: Align AArch64 and i386 image base to superpage.

Thanks for the updates. LGTM.

Wed, Sep 19, 8:04 AM

Tue, Sep 18

peter.smith added a comment to D52156: [LLD] [COFF] Alternative ARM range thunk algorithm.

Thanks for the updates. I don't have any more comments.

Tue, Sep 18, 11:13 AM

Mon, Sep 17

peter.smith added a comment to D52156: [LLD] [COFF] Alternative ARM range thunk algorithm.

My best suggestion for a test that will need exceed the margin is to use input sections with high alignment requirements. If these are on a natural boundary prior to adding thunks then adding a thunk will result in a large amount of bytes inserted to maintain section alignment. As long as these padding bytes are in between the source and destination of a branch and exceed the margin then a test case can be constructed without needing to insert hundreds of Thunks.

Mon, Sep 17, 6:08 PM
peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

Interesting. I think that the implementation you have here will converge faster as it makes it more likely that pass 1 has all the thunks, at the expense of potentially generating more thunks than is strictly required. However if the goal is simplicity I think that you'll need to do everything in one pass, and accept that there will be corner cases that might not link if the margin isn't sufficient. For ELF and arbitrary linker scripts I thought the chance of failure too high, for COFF the chance of failure may be low enough. I think the most likely edge case in COFF will be the presence of sections with high alignment requirements as inserting thunks could cause a lot of bytes of alignment padding to be added.

Mon, Sep 17, 4:53 PM
peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

Interesting. I think that the implementation you have here will converge faster as it makes it more likely that pass 1 has all the thunks, at the expense of potentially generating more thunks than is strictly required. However if the goal is simplicity I think that you'll need to do everything in one pass, and accept that there will be corner cases that might not link if the margin isn't sufficient. For ELF and arbitrary linker scripts I thought the chance of failure too high, for COFF the chance of failure may be low enough. I think the most likely edge case in COFF will be the presence of sections with high alignment requirements as inserting thunks could cause a lot of bytes of alignment padding to be added.

Mon, Sep 17, 4:19 PM
peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

If I understand it, to get a test case we'd need to have a branch that is just in range (including margin) such that no thunk is generated, but adding sufficient thunks causes that branch to go out of range. The brute force way to do it would be to generate greater than (margin/thunk-size) thunks but even with macros that would be a large tedious test to write. One possibility in ELF that I don't know would transition into COFF is to have some sections with high alignment so that inserting a thunk could displace one of these sections off an alignment boundary and hence add much more size than just the Thunk.

Mon, Sep 17, 3:34 PM
peter.smith added a comment to D51792: [AArch64] Attempt to parse expressions as adr/adrp operands.

Yes I'm happy with this too.

Mon, Sep 17, 2:32 PM
peter.smith added a comment to D50297: Align AArch64 and i386 image base to superpage.

Does FreeBsd use 4Kb granules for the page size or the LLD default of 64Kb. From what I can see from the MMU documentation when 64Kb pages are used the equivalent of the Super Page size is 512MiB and not 2 MiB? If I'm right then aligning to 2MiB is only going to be beneficial when 4Kb pages are used. On the assumption that it doesn't do much harm to people using 64Kb pages I don't have any objections in changing it though. Aligning to 512MiB for 64kb pages sounds a bit extreme though.

Mon, Sep 17, 2:30 PM
peter.smith added a reviewer for D50297: Align AArch64 and i386 image base to superpage: srhines.

I've added Stephen Hines as a reviewer to see if this affects Android in a significant way. I'd like to run this by a few people in the OS community and also check that are no adverse code/file size increases as these could be significant on low-end mobile devices. Will hopefully get back to you soon.

Mon, Sep 17, 8:40 AM

Fri, Sep 14

peter.smith added a comment to D18086: Fix default processor name for armv6k..

If no-one else beats me to it, I'll take a look when I get back from a conference (24th September).

Fri, Sep 14, 6:13 AM
peter.smith added a comment to D18086: Fix default processor name for armv6k..

I think the natural default CPU for ARMv6K is the "mpcore". The K stands for kernel extensions and includes the load,store and clear exclusive instructions but otherwise no effect on code generation. I would expect that ARMv6KZ includes the instructions for trustzone such as SMC (secure monitor call) but again, this doesn't have an effect on code generation.

Fri, Sep 14, 5:56 AM
peter.smith added a comment to D18086: Fix default processor name for armv6k..

There isn't a processor called ARM1176J-S, only ARM1176JZ-S and ARM1176JZF-S. I've already left a comment about J and S. Z is for Trustzone and F is for hardware floating point. There isn't a good external source for all the possible arm 11 CPU names unfortunately. There is a page of trademarks that contains all the ones that I know about https://www.arm.com/company/policies/trademarks/arm-trademark-list/arm11-trademark.

Fri, Sep 14, 2:16 AM

Thu, Sep 13

peter.smith created D52038: [LLD][ELF][AArch64] Guard --fix-cortex-a53-843419 against --just-syms.
Thu, Sep 13, 7:59 AM

Wed, Sep 12

peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

Sorry for the belated response. I was thinking of this patch for a while.

Every time I saw the code of thunk range extension, I wonder if we really need this multi-pass algorithm which add thunks iterative on each pass. I believe in almost all cases, the algorithm finishes on the first iteration, if we allow a very small margin when determining "reachability". As long as a margin is small, size increase by allowing a margin should be negligible.

For pathetic executables for which we need to generate tons of thunks (which enlarges distance between callers and callees and thus need multiple passes with the current algorithm), we can simply discard everything that we made in the previous iteration instead of keeping them, double the margin, and then try again from scratch. In practice, I believe that fallback doesn't happen too frequently.

What do you think of the algorithm? If it works, I prefer that algorithm because discarding everything and redo with a larger margin is simpler than keeping thunks created in previous passes.

Wed, Sep 12, 8:24 AM
peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

The algorithm looks good to me. For ELF I preferred to give the Thunk a name related to the destination as it makes it a bit easier to follow disassembled binaries, but it is not essential.

Wed, Sep 12, 6:48 AM

Mon, Sep 10

peter.smith added a comment to D51792: [AArch64] Attempt to parse expressions as adr/adrp operands.

I don't have much to add over Eli's existing comments. I think the general form of (symA - symB) + constant may not be handled well for all instructions though.

Mon, Sep 10, 8:46 AM
peter.smith added a comment to D51854: [Arm builtins] Remove non-necessary IS check.

I agree that we should be using APSR_nzcvq rather than CPSR_f as both assemble to the same instruction encoding (they disassemble to APSR_nzcvq). I think it is worth seeing if there are any objections to removing the case for ARM state msr CPSR_f, #APSR_C as it saves one instruction. Overall looking good to me though.

Mon, Sep 10, 5:59 AM

Mon, Sep 3

peter.smith added a comment to D42930: [Aarch64] Fix linker emulation for Aarch64 big endian.

Committed on behalf of Bharathi r341312.

Mon, Sep 3, 5:38 AM

Fri, Aug 31

peter.smith added a comment to D51536: [LLD] Do not set alignment to entsize for mergable sections if entsize is not a power of 2.

Changes look good to me, although I'd be happy to remove the max. The only reason I can think of for raising the alignment based on sh_entsize is that it would mean that (align n, shentsize s) could be merged into the same section (align m where m < n, shentsize s) but I'm struggling to think of a case where this would be significant. I think that if the compiler has taken advantage of the alignment of a data element or string in a SHF_MERGE section it sets the alignment of the section.

Fri, Aug 31, 8:28 AM

Thu, Aug 30

peter.smith added a comment to D48792: [ARM] Set execute-only flags in .text..

Thanks for the update. I still think this looks good to me.

Thu, Aug 30, 5:40 AM
peter.smith closed D51413: [zorg] Disable sanitzers on clang-cmake-armv8-lld.

Committed r341059.

Thu, Aug 30, 5:28 AM
peter.smith accepted D42930: [Aarch64] Fix linker emulation for Aarch64 big endian.

I've been referred back to this by a colleague trying to build with big-endian clang with binutils and is seeing an error message out of binutils. I've checked that binutils from the initial commit always used aarch64linuxb and I've verified that I cannot build a aarch64_be-linux-gnu executable with binutils without this change, and I can with this change.

Thu, Aug 30, 2:41 AM

Wed, Aug 29

peter.smith added a comment to D50141: Add errors for tiny codemodel on targets other than AArch64.

Ping. Everyone happy with this now the main part has gone in?

Wed, Aug 29, 7:02 AM
peter.smith created D51413: [zorg] Disable sanitzers on clang-cmake-armv8-lld.
Wed, Aug 29, 4:17 AM
peter.smith added a comment to D48792: [ARM] Set execute-only flags in .text..

I've added a couple of comments. One other thing to think about is the -S output from the compiler. If I compile a trivial test case with -mexecute-only I get the output:

Wed, Aug 29, 3:40 AM

Wed, Aug 22

peter.smith added a comment to D51030: [AArch64] Optimise load(adr address) to ldr address.

Thanks for looking. What is our usual response with issues like this? Is it OK for this to go ahead, as it can be used fine in some linkers. I could put it behind a flag I think, so that users have the option to turn it off (although that might not be very obvious..)

I think it depends on the context. For example:

  • How many existing builds is it going to break?
  • How easy would it be to backport the fix in binutils if it were important enough to do so?
  • Are there alternatives to binutils available?
  • Does the same fault exist when compiling the file with GCC?
Wed, Aug 22, 9:51 AM

Aug 22 2018

peter.smith added a comment to D51030: [AArch64] Optimise load(adr address) to ldr address.
  • I can't see anything obviously wrong with the BFD code for handling LO19

Trying a newer ld.bfd (2.31 from today) this is working fine in both modes, so perhaps the issue is old. It was 2.27/2.28 that I was trying earlier, but the program being compiled (timeit-target) is small enough that I didn't expect it to go out of range.

Aug 22 2018, 8:09 AM
peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

Sadly I had to resort to using .space to create large binaries. Creating the binaries is usually quick, disassembling them is unfortunately not. I tended to used gnu objdump first as that skips 0 by default to find the address ranges I needed, then used --start-address and --stop-address in llvm-objdump to pull out the bits that I need.

Aug 22 2018, 4:19 AM
peter.smith added a comment to D51089: [LLD] [COFF] Add support for creating range extension thunks for ARM.

I've left a couple of comments where I think that there may be some unintentional inefficiencies, but as far as I can tell it looks like it will be correct. I suggest adding a lot more test cases as you go for things like thunk reuse.

Aug 22 2018, 3:36 AM
peter.smith added a comment to D51032: [ARM] Avoid injecting constant islands in movw+movt pairs on Windows.

Thanks for the test update, I've no further comments.

Aug 22 2018, 2:21 AM
peter.smith added a comment to D51027: [LLD][ELD] - Do not reject INFO output section type when used with a start address..

Do you have a better source of documentation for these linker script features than the gnu docs. I find the gnu docs difficult, for example the gnu docs state for NOLOAD that: "the section should be marked as not loadable, so that it will not be loaded into memory when the program is run". Which I find difficult to interpret in in terms of ELF and leaves me required to read the source code to understand what NOLOAD does!

Aug 22 2018, 2:12 AM
peter.smith accepted D49674: [AArch64] Add Tiny Code Model for AArch64.

Looks good to me now that LLVM support has been approved in D49673

Aug 22 2018, 1:45 AM

Aug 21 2018

peter.smith added a comment to D51027: [LLD][ELD] - Do not reject INFO output section type when used with a start address..

I'm not sure it is possible to do exactly what the author is doing with https://bugs.llvm.org/show_bug.cgi?id=38625 with a section without SHF_ALLOC. The entire contents of the output section are defined by .= stack_start. As stack_start is calculated by the linker I don't think it would have been possible to do this with a section.

Aug 21 2018, 7:56 AM
peter.smith accepted D49673: [AArch64] Add Tiny Code Model for AArch64.

Looks good to me. I've not seen any other comments and I've no further comments.

Aug 21 2018, 6:38 AM
peter.smith added a comment to D51032: [ARM] Avoid injecting constant islands in movw+movt pairs on Windows.

The code changes look good to me, although someone more familiar with the ConstantIslands pass may have a better idea of how to prevent splitting. The only other thought about the test is to make a MIR test and just run the constant-islands pass. For example see the test constant-islands-cfg.mir. I suspect that it will be hard to write the test but it should be a lot less fragile.

Aug 21 2018, 6:17 AM
peter.smith added a comment to D51030: [AArch64] Optimise load(adr address) to ldr address.

Unfortunately I'm hitting a number of linker problems in testing this.

  • Armlink seems to do fine, but I am testing embedded code there.
  • ld.bfd links the test-suite fine with fpic (except for the expected errors where relocations are out of range). But without pic gives an error about relocating against stderr that I'm not sure of yet, but might point to this alignment check being wrong in some way. Unfortunately the errors are not clear what is out of range, it may be a legitimate error (although ld.gold and lld both are "successful" at linking the same program).
Aug 21 2018, 4:14 AM
peter.smith added a comment to D50998: [LLD] [COFF] Check the instructions in ARM MOV32T relocations.

For what it's worth I've checked that the bit-patterns used in the MOVT/MOVW instructions are correct. In assembler it seems like it is possible to create an error that isn't detectable at link time.

Aug 21 2018, 3:11 AM

Aug 16 2018

peter.smith updated the diff for D45961: [MC] Add MCSubtargetInfo to MCAlignFragment [NFC].

I've uploaded a new version that doesn't use getLastSubtargetInfo(). Thanks for the warning about clang tools/driver/cc1as_main.cpp I've not seen anywhere else in the project that uses InitSections(). The code there will become:

Str.get()->InitSections(Opts.NoExecStack, *STI);

As the STI has been computed earlier. I'm happy to post another review if people prefer? Otherwise I could just make the change immediately after where this to be committed.

Aug 16 2018, 10:25 AM
peter.smith added a comment to D49673: [AArch64] Add Tiny Code Model for AArch64.

From what I can see, it should just be the InitialExec model that does a LOADgot which should match the GCC behaviour with respect to TLS; so I'm happy with that for this patch. I think we can add better support for tiny code model TLS later if there is a need for it. I don't have any more comments myself, if there are no other comments from the other reviewers by the end of the week I'll LGTM.

Aug 16 2018, 8:53 AM
peter.smith added a comment to D44501: COFF: Move assignment of section offsets to assignAddresses(). NFCI..
In D44501#1200976, @pcc wrote:

This makes the design a little more similar to the ELF linker and should allow for features such as ARM range extension thunks to be implemented more easily.

@pcc - I found you mentioned this as part of this change; do you have any concrete plan on how to add ARM range extension thunks? I'm pondering giving that a try, but if you have ideas about how/where to start, that'd get me started quicker.

Have you looked at their design in the ELF port? CC @peter.smith, who added that support.

Yes, I was broadly imagining that it would work in a similar way to the ELF port.

I tried to look briefly at the commit that added the range extension thunks for ELF, but I guess I should look at more of it than that commit, since it seemed to rely on some generic thunking support that existed before. Or perhaps I should just have another look.

Aug 16 2018, 8:20 AM
peter.smith added a comment to D45961: [MC] Add MCSubtargetInfo to MCAlignFragment [NFC].
In D45961#1200623, @asb wrote:

You will also need to patch at least clang tools/driver/cc1as_main.cpp due to the change to InitSections.

Mechanically the changes seem fine. There's one aspect I'd appreciate further comment on: having to perform getLastSubtargetInfo seems pragmatic but feels ugly. Are there cases where this might be incorrect? Should there be a FIXME indicating that some superior solution for ensuring access to an appropate MCSubtargetInfo should be implemented?

Aug 16 2018, 5:42 AM
peter.smith accepted D48792: [ARM] Set execute-only flags in .text..

Thanks for the update. This looks good to me. It may be worth waiting a few days before committing to see if there are any objections/other opinions given that this touches some code shared between multiple backends.

Aug 16 2018, 3:24 AM

Aug 14 2018

peter.smith added a comment to D49673: [AArch64] Add Tiny Code Model for AArch64.

Generally looking good to me. Apologies for the delay.

Aug 14 2018, 3:50 AM
peter.smith added a comment to D50077: [LLD][ELF][ARM] Add support for Armv5 and Armv6 compatible Thunks.

Ping, I don't think that there are any outstanding comments that need addressing.

Aug 14 2018, 3:18 AM
peter.smith added a comment to D50076: [LLD][ELF][ARM] Add support for older Arm Architectures with smaller branch range.

Ping, I don't think that there are any outstanding comments that need addressing.

Aug 14 2018, 3:17 AM
peter.smith added a comment to D50688: [LLD] Warn if non-alloc sections occur before alloc sections in linker scripts.

Interesting. This seems to be one of the situations where linker scripts aren't specified well enough to give a definitive answer of what a linker should do.

Aug 14 2018, 3:04 AM

Aug 10 2018

peter.smith accepted D50459: [zorg] Cleanup clang-cmake-aarch64-lld and clang-cmake-armv8-lld bots.

Approving as these are linaro buildbot specific.

Aug 10 2018, 5:49 AM
peter.smith accepted D50456: [zorg] Add AArch32 Linux LLD buildbot.

Approving as these are linaro buildbot specific.

Aug 10 2018, 5:49 AM
peter.smith accepted D50460: [zorg] Increase parallelism for bots running on 96-core server..

LGTM too

Aug 10 2018, 5:48 AM
peter.smith accepted D50458: [zorg] Update configuration of Linaro's AArch64 bots.

LGTM too.

Aug 10 2018, 5:48 AM

Aug 9 2018

peter.smith added a comment to D50459: [zorg] Cleanup clang-cmake-aarch64-lld and clang-cmake-armv8-lld bots.

Looks good to me, we've tested this out internally.

Aug 9 2018, 5:33 AM
peter.smith added a comment to D50456: [zorg] Add AArch32 Linux LLD buildbot.

Looks good to me, this matches the configuration we've had internally at Linaro.

Aug 9 2018, 5:33 AM

Aug 8 2018

peter.smith created D50440: [LLD] Add missing REQUIRES x86 to tests..
Aug 8 2018, 5:45 AM
peter.smith added a comment to D48792: [ARM] Set execute-only flags in .text..

I've left some brief comments. As this is hitting the generic parts of MC it may be worth casting the net wider to look for some more reviewers?

Aug 8 2018, 3:29 AM

Aug 3 2018

peter.smith updated the diff for D50234: [LLD][ELF][ARM] Test undefined weak symbol for Thumb narrow branch.

Thanks for the comments. I've updated the diff.

Aug 3 2018, 7:27 AM
peter.smith updated the diff for D50234: [LLD][ELF][ARM] Test undefined weak symbol for Thumb narrow branch.

Thanks for the suggestion to use yaml2obj. I keep forgetting about that. I've been able to make the same test case.

Aug 3 2018, 7:01 AM
peter.smith updated the diff for D50234: [LLD][ELF][ARM] Test undefined weak symbol for Thumb narrow branch.

I've updated the comment to mention the gcc version. The code for the object is in the comment, I've made that a bit clearer.

Aug 3 2018, 4:20 AM
peter.smith added a comment to D26240: [LLD][ARM][AArch64] ARM and AArch64 undefined weak reference values for relative relocations.

I've proposed a test case in https://reviews.llvm.org/D50234 it does need a binary file as llvm-mc does indeed relax the b.n into a b.w.

Aug 3 2018, 3:04 AM
peter.smith created D50234: [LLD][ELF][ARM] Test undefined weak symbol for Thumb narrow branch.
Aug 3 2018, 3:03 AM

Aug 2 2018

peter.smith added inline comments to D26240: [LLD][ARM][AArch64] ARM and AArch64 undefined weak reference values for relative relocations.
Aug 2 2018, 7:19 AM

Aug 1 2018

peter.smith added a comment to D50141: Add errors for tiny codemodel on targets other than AArch64.

I don't have any better suggestions about where to move the function right now. I have a couple of minor suggestions.

Aug 1 2018, 8:27 AM
peter.smith updated the diff for D50077: [LLD][ELF][ARM] Add support for Armv5 and Armv6 compatible Thunks.

Updated patch. Changes:

  • Correct instruction sequence in PI thunk to avoid use of Armv7 instruction
  • Update and add tests for error messages
  • Moved ThunkSectionSpacing into a function
Aug 1 2018, 7:06 AM
peter.smith added a comment to D50077: [LLD][ELF][ARM] Add support for Armv5 and Armv6 compatible Thunks.

Thanks for the comments, I'll post another patch shortly.

Aug 1 2018, 6:46 AM

Jul 31 2018

peter.smith created D50077: [LLD][ELF][ARM] Add support for Armv5 and Armv6 compatible Thunks.
Jul 31 2018, 9:23 AM
peter.smith created D50076: [LLD][ELF][ARM] Add support for older Arm Architectures with smaller branch range.
Jul 31 2018, 9:02 AM
peter.smith accepted D49547: [ELF] - Get rid of postThunkContents()..

LGTM. Probably worth waiting a bit to see if Rui has any opinions.

Jul 31 2018, 6:01 AM
peter.smith added a comment to D49673: [AArch64] Add Tiny Code Model for AArch64.

One example where gcc (7.1 in my case) will generate different code for tls is:

Jul 31 2018, 5:45 AM
peter.smith added a comment to D49547: [ELF] - Get rid of postThunkContents()..

Looks reasonable to me. I've made a suggestion that might simplify a bit further, I've not tried or tested it though.

Jul 31 2018, 5:27 AM
peter.smith added a comment to D50049: [ARM] Complete enumeration values for Tag_ABI_VFP_args.

Thanks, I'll update the patches to use ToolChain on commit.

Jul 31 2018, 5:25 AM
peter.smith updated the diff for D49993: [LLD][ELF][ARM] Implement support for Tag_ABI_VFP_args.

Thanks all for the comments.

Jul 31 2018, 4:01 AM
peter.smith created D50049: [ARM] Complete enumeration values for Tag_ABI_VFP_args.
Jul 31 2018, 3:55 AM
peter.smith added a comment to D49822: [GlobalMerge] Allow merging globals with explicit section markings..

I think this looks fine from an ELF perspective. As I understand it the default section names like .data and .bss are more convention than a requirement so it shouldn't matter what the name of the section is.

Jul 31 2018, 2:31 AM
peter.smith accepted D49936: [ARM] Support the .isnt directive for MachO and COFF targets.

@peter.smith, yeah, I think that sharing this might make it more complex. This looks fine to me, any other concerns?

Jul 31 2018, 1:43 AM

Jul 30 2018

peter.smith created D49993: [LLD][ELF][ARM] Implement support for Tag_ABI_VFP_args.
Jul 30 2018, 9:20 AM
peter.smith created D49992: [ELF][ARM] Add Arm ABI names for float ABI ELF Header flags.
Jul 30 2018, 9:02 AM
peter.smith added inline comments to D49936: [ARM] Support the .isnt directive for MachO and COFF targets.
Jul 30 2018, 5:55 AM
peter.smith accepted D47217: [cmake] [ARM] Exclude any VFP builtins if VFP is not supported.

Apologies for the delay. This looks good to me. I've checked that I can build v7-m without floating point support and I've checked that I can build and run the tests for armv7-linux-gnueabihf.

Jul 30 2018, 5:44 AM
peter.smith added a comment to D49936: [ARM] Support the .isnt directive for MachO and COFF targets.

I can't comment too much on the use of .inst in machO and Windows. I have a small suggestion that might make it possible to share more of the implementation, but it is only a personal preference.

Jul 30 2018, 3:52 AM
peter.smith added a comment to D49937: [ARM] Allow automatically deducing the thumb instruction size for .inst.

That looks like the right behaviour to me. For reference the Arm ARM definition of Thumb Instruction width from encodeing is in https://developer.arm.com/products/architecture/cpu-architecture/a-profile/docs/ddi0406/latest/arm-architecture-reference-manual-armv7-a-and-armv7-r-edition 6.1 Thumb Instruction set encoding.

Jul 30 2018, 3:24 AM

Jul 25 2018

peter.smith added a comment to D48518: ADT: Shrink SmallVector by 8B on 64-bit platforms.

This patch is causing the http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld to fail. Unfortunately (for me) the failures seems to be restricted to older versions of clang and are not reproducible on Arm or X86. The buildbot itself is using the default /usr/bin/clang for Ubuntu 16.04 LTS (clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final). The failures go away if I use the 3.9.1 release (No failures on 4.0.1, 6.0.0 or built from master). At this stage I think that this could be a code-gen bug in the AArch64 backend of clang 3.8.1 so I don't think it is worth reverting, but I'm mentioning it here just in case there is a subtle problem that is only being exposed by that version.

Jul 25 2018, 10:32 AM

Jul 24 2018

peter.smith added a comment to D49673: [AArch64] Add Tiny Code Model for AArch64.

I think that it is important to support position independent code in the tiny code model, there was a thread on llvm-dev in May where someone couldn't 4k align his position independent code on a bare-metal AArch64 platform (http://lists.llvm.org/pipermail/llvm-dev/2018-May/123465.html) A tiny code model that avoids ADRP could be a solution there. A PR was raised to implement it https://bugs.llvm.org/show_bug.cgi?id=37543 . If we can get position independent support added then it will be worth closing that PR. I guess it could be added in a follow up patch though.

Jul 24 2018, 8:28 AM
peter.smith added a comment to D49716: ELF: Do not ICF SHF_LINK_ORDER sections..

I definitely think that it is the right thing to do to consider the section and the SHF_LINK_ORDER section as a unit. I'm assuming that no merging of the code sections is done if the SHF_LINK_ORDER sections are different.

Jul 24 2018, 3:24 AM

Jul 18 2018

peter.smith added a reviewer for D49456: [AArch64] Support execute-only LOAD segments.: ruiu.

Adding Rui as code owner as he'll probably want to sign off on the introduction of a command line option.

Jul 18 2018, 5:52 AM

Jul 17 2018

peter.smith added a comment to D47217: [cmake] [ARM] Exclude any VFP builtins if VFP is not supported.

Can you elaborate on why the armhf buildbot failed and why you have excluded armhf from the MATCHES expression above. A simple test of a source file containing

Jul 17 2018, 9:52 AM
peter.smith accepted D49371: [ELF] - Do not produce broken output when amount of sections is > ~65k.

Looks good to me thanks.

Jul 17 2018, 5:43 AM
peter.smith added a comment to D49371: [ELF] - Do not produce broken output when amount of sections is > ~65k.

This looks correct to me. I've got a few small suggestions.

Jul 17 2018, 5:22 AM
peter.smith added a comment to D49407: Add a relocation for R_AARCH64_ABS32 in ObjectFileELF.

Speaking as primarily a linker person; the implementation of the R_AARCH64_ABS32 relocation is correct. You'll need to get some input from an LLDB person about whether extending the function like this is the right approach though, I'm guessing that there are more than just AArch64 and Arm that support relocated debug sections? The more codes that are added the less readable that assert is going to get. Final comment: is there a test case that you can add to show that R_AARCH64_ABS32 is handled correctly, or even just doesn't assert fail?

Jul 17 2018, 3:55 AM

Jul 6 2018

peter.smith accepted D48931: [Driver,AArch64] Add support for -mcpu=native..

Looks good to me.

Jul 6 2018, 2:01 AM · Restricted Project

Jul 4 2018

peter.smith added a comment to D48931: [Driver,AArch64] Add support for -mcpu=native..

Looks ok to me. I've checked that gcc also supports -mcpu=native on an AArch64 machine. It also supports -march=native. While I don't think that this is necessary to support it for this patch is it worth considering as well?

Jul 4 2018, 7:48 AM · Restricted Project
peter.smith added a comment to D48791: [AArch64] Implement execute-only CodeGen Support..

Some early feedback from the AArch64 ABI along the lines of "Do you actually need SHF_AARCH64_PURECODE?" In Arm there has been a long history of using literal pools so it is important to mark code that has been compiled to avoid them as SHF_ARM_PURECODE. In AArch64 the expectation is that code is execute-only unless someone has forced it otherwise. An alternative model is to assume that all executable sections are implicitly execute-only and therefore all that you would need is an option at link time to remove PF_R from a program header that contained only executable sections. In essence we can have a one time declare all executable sections as execute-only at link time. I think that this would be compatible with adding SHF_AARCH64_PURECODE later if there was a code-generation model that made use of literal pools.

Jul 4 2018, 2:43 AM

Jul 3 2018

peter.smith added a comment to D48792: [ARM] Set execute-only flags in .text..

As an example for LTO, or just using a single bitcode file I'm thinking of something like

Jul 3 2018, 9:38 AM

Jul 2 2018

peter.smith added a comment to D48792: [ARM] Set execute-only flags in .text..

I think that setting SHF_ARM_PURECODE for the initial .text section is the right thing to do; the flag is a declaration that the section does not read from the .text section, and that property is satisfied by an empty section. For reference the code you've restored was removed in rL306927 "Rewrite ARM execute only support to avoid the use of a command line flag and unqualified ARMSubtarget lookup." I think the unqualified ARMSubtarget lookup might need a bit more exploration; for example what happens if this is the link step of a LTO build where the functions have ARMSubtargets with SHF_ARM_PURECODE but the command line to the link step has no code generation flags I think that the ARMSubtarget is likely to not have the ARM::FeatureExecuteOnly. If this is the case then we may need an alternative approach. At a glance would it be possible to always add SHF_ARM_PURECODE to the initial .text section?

Jul 2 2018, 10:44 AM
peter.smith created D48840: [LLD][ELF][AArch64] Add test cases for load/store exclusive instructions [NFC].
Jul 2 2018, 9:54 AM
peter.smith added a comment to D48795: [AArch64] Add support for SHF_ARM_PURECODE..

I've left a comment about the ABI problems with using SHF_ARM_PURECODE. I've raised an issue to support execute only on AArch64 with the ABI team at Arm.

Jul 2 2018, 4:16 AM
peter.smith added a comment to D48791: [AArch64] Implement execute-only CodeGen Support..

There is an ABI issue with using an Arm specific flag that isn't defined in AArch64. I've not had a chance to check over the code yet, but will try to do so later today.

Jul 2 2018, 2:48 AM

Jun 29 2018

peter.smith added a comment to D36742: [LLD][ELF][AArch64] Implement scanner for Cortex-A53 Erratum 843419.

I'll come up with something. Will aim to do it on Monday. The load/store exclusive instructions are architecture v8.1 only so aren't supported by the v8.0 cortex-a53; it is unlikely that anyone will run the erratum fix that only affects cortex-a53 with the option but it should be possible to come up with test case.

Jun 29 2018, 3:36 AM

Jun 26 2018

peter.smith closed D48077: [LNT] Allow --use-perf=profile and --run-under to work together.

Committed under r335463 my apologies I forgot the Differential Revision commit that would automatically close this revision.

Jun 26 2018, 2:04 AM

Jun 15 2018

peter.smith added a comment to D48146: ELF: Implement --icf=safe using address-significance tables..

Thanks very much for the update. I don't have any more comments at this stage.

Jun 15 2018, 1:25 AM

Jun 14 2018

peter.smith added a comment to D45959: [MC] Move bundling and MCSubtargetInfo to MCEncodedFragment [NFC].

Thanks, I'll wait till tomorrow before committing to see if anyone in the US timezone has any objections.

Jun 14 2018, 8:56 AM