Page MenuHomePhabricator

MaskRay (Fangrui Song)
UserAdministrator

Projects

User Details

User Since
Dec 30 2016, 3:24 PM (211 w, 5 d)
Roles
Administrator

Recent Activity

Today

MaskRay committed rG71635ea5ffd6: MCDwarf: Delete uneeded parameter (authored by MaskRay).
MCDwarf: Delete uneeded parameter
Thu, Jan 21, 12:55 AM

Yesterday

MaskRay added inline comments to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.
Wed, Jan 20, 11:09 PM · Restricted Project
MaskRay added inline comments to D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter.
Wed, Jan 20, 11:06 PM · Restricted Project
MaskRay updated the diff for D93190: [libc++abi] Simplify scan_eh_tab.

forgot to include a paren change in the previous diff

Wed, Jan 20, 6:02 PM · Restricted Project
MaskRay updated the diff for D93190: [libc++abi] Simplify scan_eh_tab.

fix an assert and comment

Wed, Jan 20, 6:00 PM · Restricted Project
MaskRay updated the diff for D93190: [libc++abi] Simplify scan_eh_tab.

Add assert

Wed, Jan 20, 5:53 PM · Restricted Project
MaskRay committed rG6afdf13ae4cc: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive… (authored by MaskRay).
Makefile.rules: Avoid redundant .d generation (make restart) and inline archive…
Wed, Jan 20, 2:22 PM
MaskRay closed D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.
Wed, Jan 20, 2:22 PM · Restricted Project
MaskRay updated the diff for D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

$(RM) a.o b.o

Wed, Jan 20, 2:20 PM · Restricted Project
MaskRay updated the diff for D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Remove more ARCHIVE_NAME

Wed, Jan 20, 1:04 PM · Restricted Project
MaskRay added a comment to D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Is this good? :)

I think you forgot to update the patch?

Wed, Jan 20, 1:00 PM · Restricted Project
MaskRay updated the diff for D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Inline archive rule

Wed, Jan 20, 12:59 PM · Restricted Project
MaskRay added a comment to D94882: [MC] Upgrade DWARF version to 5 upon .file 0.

Okay, I'm persuaded by the arguments. I don't really mind whether the tool will ignore an explicitly specified version or not, as long as a warning is emitted if it does ignore it. I think we want to avoid the potential confusion of "I asked for DWARFv3, but am getting DWARFv5 output", and a warning would at least help with that (assuming of course we don't just refuse to handle this case, which I think is also an acceptable, and possibly preferable solution).

Wed, Jan 20, 11:54 AM · Restricted Project
MaskRay added a comment to D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Is this good? :)

Wed, Jan 20, 11:06 AM · Restricted Project
MaskRay added inline comments to D94925: [lld] Consistent help text for `-save-temps`.
Wed, Jan 20, 10:41 AM · Restricted Project
MaskRay added a comment to D94925: [lld] Consistent help text for `-save-temps`.

BTW, do you know if we can remove the rules that add espindola (who has left the project IIUC) from all lld/ELF changes?

Wed, Jan 20, 10:40 AM · Restricted Project
MaskRay retitled D83154: clang: Add -fprofile-prefix-map from clang: Add -fcoverage-prefix-map to clang: Add -fprofile-prefix-map.
Wed, Jan 20, 10:37 AM · Restricted Project
MaskRay requested changes to D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section.
Wed, Jan 20, 10:35 AM · Restricted Project
MaskRay committed rG36e62b1ff7e7: [AArch64] Fix -Wunused-but-set-variable in GCC -DLLVM_ENABLE_ASSERTIONS=off… (authored by MaskRay).
[AArch64] Fix -Wunused-but-set-variable in GCC -DLLVM_ENABLE_ASSERTIONS=off…
Wed, Jan 20, 10:14 AM
MaskRay accepted D94907: [llvm-nm][ELF] - Make -D display symbol versions..

LGTM.

Wed, Jan 20, 9:36 AM · Restricted Project
MaskRay added inline comments to D95056: [CSSPGO] LTO option for pseudo probe.
Wed, Jan 20, 9:30 AM · Restricted Project

Tue, Jan 19

MaskRay committed rGf96ff3c0f8eb: [ELF] --wrap: Produce a dynamic symbol for undefined __wrap_ (authored by MaskRay).
[ELF] --wrap: Produce a dynamic symbol for undefined __wrap_
Tue, Jan 19, 9:24 PM
MaskRay committed rG8031785f4a7e: [ELF][test] Improve --wrap tests (authored by MaskRay).
[ELF][test] Improve --wrap tests
Tue, Jan 19, 9:24 PM
MaskRay accepted D94611: MC: AArch64: Add support for gotpage_lo15.
Tue, Jan 19, 1:21 PM · Restricted Project
MaskRay added a comment to D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Looks like a nice cleanup. The only part I am not sure of is the part about removing $(RM) $(ARCHIVE_OBJECTS). Is that necessary?
I'm not sure why is that line there, but if I had to guess, I would say it's to ensure that lldb (on macos) reads debug info from the archive file instead of the original .o files. If it's not required, it may be better to leave it in. Otherwise, someone from Apple should say whether that is ok (testing archives is only really interesting on fruity platforms).

I can add back it under the ifeq "$(OS)" "Darwin" guard if Apple folks think it is useful.

It looks like we have only one test on llvm.org (+ one additional test in the Swift fork) that's using this. My vote is to just remove this together with the ARCHIVE_C_SOURCES, ARCHIVE_CXX_SOURCES, ARCHIVE_OBJC_SOURCES and ARCHIVE_OBJCXX_SOURCES and inline it in that one test.

Tue, Jan 19, 1:09 PM · Restricted Project
MaskRay accepted D94853: [NFC] Disallow unused prefixes under Other.

LGTM. If --allow-unused-prefixes=false tests are more than --allow-unused-prefixes=true tests, it is probably time to flip FileCheck default.

Tue, Jan 19, 12:22 PM · Restricted Project
MaskRay added a comment to D94882: [MC] Upgrade DWARF version to 5 upon .file 0.

A few thoughts:

  1. If gnu-as already has accepted such a patch, we probably want to do the same just for compatibility
Tue, Jan 19, 12:18 PM · Restricted Project
MaskRay committed rG5fcb412ed083: [ELF] Support R_PPC64_ADDR16_HIGH (authored by MaskRay).
[ELF] Support R_PPC64_ADDR16_HIGH
Tue, Jan 19, 11:43 AM
MaskRay committed rGe12e0d66c03c: [ELF] Error for out-of-range R_PPC64_ADDR16_HA, R_PPC64_ADDR16_HI and their… (authored by MaskRay).
[ELF] Error for out-of-range R_PPC64_ADDR16_HA, R_PPC64_ADDR16_HI and their…
Tue, Jan 19, 11:43 AM
MaskRay committed rGd39adeaf440b: [ELF] Improve R_PPC64_ADDR* relocation tests (authored by MaskRay).
[ELF] Improve R_PPC64_ADDR* relocation tests
Tue, Jan 19, 11:43 AM
MaskRay added a comment to D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Looks like a nice cleanup. The only part I am not sure of is the part about removing $(RM) $(ARCHIVE_OBJECTS). Is that necessary?
I'm not sure why is that line there, but if I had to guess, I would say it's to ensure that lldb (on macos) reads debug info from the archive file instead of the original .o files. If it's not required, it may be better to leave it in. Otherwise, someone from Apple should say whether that is ok (testing archives is only really interesting on fruity platforms).

Tue, Jan 19, 9:49 AM · Restricted Project
MaskRay added a comment to D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber.

I'll leave this one to @aprantl

Tue, Jan 19, 9:15 AM · Restricted Project
MaskRay updated the diff for D94888: [lldb] Add -Wl,-rpath to make tests run with fresh built libc++.

Delete if os.path.isdir("/usr/include/c++/v1"):

Tue, Jan 19, 9:14 AM · Restricted Project
MaskRay added a comment to D93190: [libc++abi] Simplify scan_eh_tab.

@compnerd is already part of the libc++abi group, but I'm also adding him explicitly in case he has the time to look, since he has a lot of relevant experience.

Tue, Jan 19, 9:10 AM · Restricted Project
MaskRay accepted D94481: [llvm-symbolizer][doc] Reorder --relativenames in options list.
Tue, Jan 19, 9:10 AM · Restricted Project
MaskRay added a comment to D94882: [MC] Upgrade DWARF version to 5 upon .file 0.

Maybe a silly suggestion (and one that would need more discussion on the mailing lists), but why not just change the default version to 5 more widely? How often do people deliberately deviate from the default DWARF version in practice? To me, the proposed change feels like a bit of a hack for what is likely fairly unusual usage, and is only temporary, since likely most toolchains will want to change to DWARF v5 at some point going forward.

ie: you're suggesting users needing to use DWARFv4 line tables would access that functionality with a flag passed to the assembler (as they do today if they want to opt in to DWARFv5)?

Right, yes, that's what I'm suggesting.

I think part of the problem is that people aren't used to (ie: build systems and other infrastructure isn't designed for) having to pass a dwarf version to their assembler - the line table format hasn't changed since DWARFv2. So if you have run -gN -S, you can pass the resulting file to the compiler/assembler without any flags. So even if we switch the default - then people who aren't ready to switch to DWARFv4 are going to be in a possibly difficult situation of trying to teach their build system how to pass the appropriate flags to the assembler, etc.

I think maybe the way to think about it is that there wouldn't be a default DWARF version - it could always be determined by the presence/absence of a file 0.

I'm concerned about relying on what is essentially a minor detail of the specification, for a couple of reasons:

  1. What if the .file 0 directive isn't present for future DWARF versions (e.g. DWARFv6), for whatever reason? You might have some code that is generated expecting DWARFv6, yet the assembler will end up generating (or at least trying to generate) DWARFv4 output. It seems like relying on a detail like this is fragile and not particularly future-proof.
Tue, Jan 19, 12:44 AM · Restricted Project
MaskRay added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Tue, Jan 19, 12:27 AM · Restricted Project
MaskRay added a comment to D94888: [lldb] Add -Wl,-rpath to make tests run with fresh built libc++.

It looks like this is removing the ability to build libc++ tests with gcc (as it does not have the -stdlib option). While having that ability would be nice, I don't believe there's anyone currently using that configuration, so it shouldn't stand in the way of other things. But we should also update the python detection code then (in canRunLibcxxTests in packages/Python/lldbsuite/test/dotest.py -- I guess you just need to remove the if os.path.isdir("/usr/include/c++/v1"): blurb)

As for testing against the system libc++ with clang, I guess that should still work, as the extra rpath will be just ignored in that case...

Tue, Jan 19, 12:19 AM · Restricted Project

Mon, Jan 18

MaskRay added inline comments to D94611: MC: AArch64: Add support for gotpage_lo15.
Mon, Jan 18, 12:36 PM · Restricted Project
MaskRay updated the diff for D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.

Don't use double-colon. https://lists.gnu.org/archive/html/help-make/2021-01/msg00016.html

Mon, Jan 18, 9:42 AM · Restricted Project

Sun, Jan 17

MaskRay committed rGb74ae43c44b1: Makefile.rules: Make HOST_OS/OS simply expanded variable to avoid excess uname… (authored by MaskRay).
Makefile.rules: Make HOST_OS/OS simply expanded variable to avoid excess uname…
Sun, Jan 17, 5:19 PM
MaskRay updated the summary of D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.
Sun, Jan 17, 5:02 PM · Restricted Project
MaskRay requested review of D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test.
Sun, Jan 17, 5:01 PM · Restricted Project
MaskRay committed rG95d146182fdf: Makefile.rules: Delete GCC 4.6 workaround (authored by MaskRay).
Makefile.rules: Delete GCC 4.6 workaround
Sun, Jan 17, 1:17 PM
MaskRay requested review of D94888: [lldb] Add -Wl,-rpath to make tests run with fresh built libc++.
Sun, Jan 17, 12:40 PM · Restricted Project
MaskRay added a comment to D94882: [MC] Upgrade DWARF version to 5 upon .file 0.

Theoretically the diagnostic can be retained: we need another variable beside MCContext::DwarfVersion or making MCContext::DwarfVersion Optional<int>; and carefully doing similar thing on Clang cc1as side.

Sun, Jan 17, 10:50 AM · Restricted Project
MaskRay requested review of D94882: [MC] Upgrade DWARF version to 5 upon .file 0.
Sun, Jan 17, 10:40 AM · Restricted Project
MaskRay committed rG3809f4ebabde: [ELF] Support R_PPC_ADDR24 (ba foo; bla foo) (authored by MaskRay).
[ELF] Support R_PPC_ADDR24 (ba foo; bla foo)
Sun, Jan 17, 12:02 AM

Sat, Jan 16

MaskRay committed rGa048ce13e32d: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or… (authored by MaskRay).
[X86] Default to -x86-pad-for-align=false to drop assembler difference with or…
Sat, Jan 16, 4:40 PM
MaskRay closed D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.
Sat, Jan 16, 4:40 PM · Restricted Project

Fri, Jan 15

MaskRay added inline comments to D94853: [NFC] Disallow unused prefixes under Other.
Fri, Jan 15, 6:43 PM · Restricted Project
MaskRay updated the diff for D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.

comments

Fri, Jan 15, 4:41 PM · Restricted Project
MaskRay added inline comments to D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.
Fri, Jan 15, 4:28 PM · Restricted Project
MaskRay added inline comments to D94560: [ELF] report section sizes when output file too large.
Fri, Jan 15, 4:23 PM · Restricted Project
MaskRay accepted D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.

Thanks for looking into llvm-nm -D😊

Fri, Jan 15, 2:45 PM · Restricted Project
MaskRay added inline comments to D94560: [ELF] report section sizes when output file too large.
Fri, Jan 15, 2:23 PM · Restricted Project
MaskRay added a comment to D94560: [ELF] report section sizes when output file too large.

You may add tests to test/ELF/linkerscript/output-too-large.s. You'll need at least another output section to demonstrate the effects.

Fri, Jan 15, 2:22 PM · Restricted Project
MaskRay added a comment to D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.

The value of -mbranches-within-32B-boundaries also dilutes over time (it mitigates issues for some Skylake architectures).
(Surprising to me, this feature has been available for one year but I don't see a lot of adoption).

The performance impact of the microcode fix were highly variable depending on the exact details of each workload. Mostly barely moved, some really suffered. I know of a couple of organizations using the mitigation, but I agree with your general point about this being something that decays in value with time. Hopefully, in a few years, we can stop talking about this.

Fri, Jan 15, 1:49 PM · Restricted Project
MaskRay added a comment to D94670: [DebugInfo][NFC] add a new DIE type to represent label + offset.

I want to have a feeling how the aix assembly works, so I tried binutils-gdb:

Fri, Jan 15, 11:03 AM · Restricted Project
MaskRay accepted D94809: [LLD][ELF][AArch64] Set _GLOBAL_OFFSET_TABLE_ at the start of .got.
aarch64-linux-gnu-gcc -nostdlib -shared a.c -fpic -o a.so
clang -target aarch64-linux-gnu -fuse-ld=lld -nostdlib -shared a.c -fpic -o a.so
Fri, Jan 15, 10:38 AM · Restricted Project

Thu, Jan 14

MaskRay updated the summary of D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.
Thu, Jan 14, 10:12 PM · Restricted Project
MaskRay updated the diff for D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.

Drop DW_AT_decl_file as well

Thu, Jan 14, 10:11 PM · Restricted Project
MaskRay added a comment to D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.

https://bugs.llvm.org/show_bug.cgi?id=42138 was a BranchFolding bug which was probably found by an automating framework. @condy reopened it in #c13 because the symptom looks similar.
I closed 42138 in favor of the X86AsmBackend dedicated https://bugs.llvm.org//show_bug.cgi?id=48742 .

Thu, Jan 14, 9:58 PM · Restricted Project
MaskRay retitled D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location from CGDebugInfo: Drop unneeded line number for FlagFwdDecl declarations to CGDebugInfo: Drop unneeded line number for CreatedLimitedType.
Thu, Jan 14, 8:55 PM · Restricted Project
MaskRay added a comment to D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.

Ah, seems I was confused by the patch title - it looks like these values aren't used for FlagFwdDecl records - forward declarations bail out 3356, not using the file/line created on 3338/3339.

Seems like this change might lead to a situation of files but no line numbers, which seems a bit strange to me - well, at least for __va_list_tag, which isn't anymore associated with the file than with the line. Perhaps both should be addressed?

Thu, Jan 14, 8:51 PM · Restricted Project
MaskRay added inline comments to D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.
Thu, Jan 14, 7:11 PM · Restricted Project
MaskRay added a comment to D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.

diff

-  [30] .debug_abbrev     PROGBITS        0000000000000000 ec7533c 616a22 00      0   0  1
-  [31] .debug_info       PROGBITS        0000000000000000 f28bd5e 50f9770b 00      0   0  1
-  [32] .debug_ranges     PROGBITS        0000000000000000 60223469 285e9b0 00      0   0  1
-  [33] .debug_str        PROGBITS        0000000000000000 62a81e19 ce211e0 01  MS  0   0  1
-  [34] .debug_line       PROGBITS        0000000000000000 6f8a2ff9 8a5ea2d 00      0   0  1
-  [35] .symtab           SYMTAB          0000000000000000 78301a28 156db60 18     37 821346  8
-  [36] .shstrtab         STRTAB          0000000000000000 7986f588 000168 00      0   0  1
-  [37] .strtab           STRTAB          0000000000000000 7986f6f0 69ab553 00      0   0  1
+  [30] .debug_abbrev     PROGBITS        0000000000000000 ec7533c 618ce4 00      0   0  1
+  [31] .debug_info       PROGBITS        0000000000000000 f28e020 50f97700 00      0   0  1
+  [32] .debug_ranges     PROGBITS        0000000000000000 60225720 285e9b0 00      0   0  1
+  [33] .debug_str        PROGBITS        0000000000000000 62a840d0 ce211e0 01  MS  0   0  1
+  [34] .debug_line       PROGBITS        0000000000000000 6f8a52b0 8a5ea2d 00      0   0  1
+  [35] .symtab           SYMTAB          0000000000000000 78303ce0 156db60 18     37 821346  8
+  [36] .shstrtab         STRTAB          0000000000000000 79871840 000168 00      0   0  1
+  [37] .strtab           STRTAB          0000000000000000 798719a8 69ab553 00      0   0  1
Thu, Jan 14, 5:38 PM · Restricted Project
MaskRay updated the summary of D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.
Thu, Jan 14, 5:36 PM · Restricted Project
MaskRay requested review of D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location.
Thu, Jan 14, 5:31 PM · Restricted Project
MaskRay updated subscribers of D93881: [llvm-objcopy] preserve file ownership when overwritten.

But I think we should do better: instead of: (1) create a temp file (2) open it by name again (3) check it is a regular file with one hard link & it is writable (S_IWUSR) (4) fstat (5) fchown,
we should just modify the destination in place (for objcopy file and strip file). If the file has been mapped readonly, on many platforms it may be unwritable. We have to accept this.

@MaskRay I took a look at the how ELF binaries are handled and from what I understand, the code directly copies the content of the input file into temporary file (https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/Object.cpp#L1786), before replacing the output file with it. So unless we are willing to allocate memory space to save its content (which will be very costive for large binaries), it does not appear modifying the input file in place is easy. And if we ever decide to take that path, it looks like we would have to make a lot of changes as the code of interest is buried deeply in the call hierarchy, and in addition we would likely have to change how the other file formats handle such cases to be consistent. Please let me know if I misunderstood your proposal or overlooked anything, but so far it appears to me the best place to preserve the file ownership without major change to the code is in the proposed place.

Thu, Jan 14, 3:25 PM · Restricted Project
MaskRay added a comment to D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.

When -mbranches-within-32B-boundaries (to mitigate microcode update for Intel JCC Erratum) is used, there are many alignment fragments. I think D75203 has advantage in that case.
In the absence of -mbranches-within-32B-boundaries, I don't think there is demonstrable improvement. I think the assembler relaxation does not pull its weight.
I'll wait another two days.

Thu, Jan 14, 2:53 PM · Restricted Project
MaskRay updated the summary of D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.
Thu, Jan 14, 2:52 PM · Restricted Project
MaskRay updated subscribers of D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.
Thu, Jan 14, 2:50 PM · Restricted Project
MaskRay added inline comments to D91583: [LTO] Prevent devirtualization for symbols exported to dynamic linker.
Thu, Jan 14, 2:24 PM · Restricted Project
MaskRay accepted D88438: BreakCriticalEdges: do not split the critical edge from a CallBr indirect successor.

LGTM.

Thu, Jan 14, 2:00 PM · Restricted Project
MaskRay added inline comments to D88438: BreakCriticalEdges: do not split the critical edge from a CallBr indirect successor.
Thu, Jan 14, 1:29 PM · Restricted Project
MaskRay added a comment to D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.

Can we not just make -gsplit-dwarf set the DwarfFissionKind without also looking at other things? That way, we don't have to worry about detecting the right mix of -g, -g2, -fthinlto-index, -x ir, bitcode input, etc. If there is some reason why this doesn't work, I would also like it to be expressed in a comment in the code so that someone doesn't come along and simplify/break the code later on.

Thu, Jan 14, 12:26 PM · Restricted Project
MaskRay accepted D92403: [LSan][RISCV] Enable LSan for RISCV64.
Thu, Jan 14, 12:10 PM · Restricted Project, Restricted Project
MaskRay committed rGe3b9af92a482: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input (authored by MaskRay).
[Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input
Thu, Jan 14, 11:46 AM
MaskRay closed D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input.
Thu, Jan 14, 11:46 AM · Restricted Project
MaskRay updated the summary of D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input.
Thu, Jan 14, 11:44 AM · Restricted Project
MaskRay added a comment to D66340: [RISCV] Support NonLazyBind.

-fno-plt essentially inlines the PLT entry into the call site. For x86-64, there are benefits if many call sites are defined in different DSOs.

Thu, Jan 14, 10:57 AM · Restricted Project
MaskRay added a comment to D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input.

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

I tried replacing if (IRInput || Args.hasArg(options::OPT_g_Group)) { with if (1), -gsplit-dwarf may produce .dwo for regular non-g .c compile.

Are you saying that if you make that change -gsplit-dwarf does cause .dwo files to be created for non-g .c compiles? Do the dwo files have anything in them? I had modified llvm to dynamically choose split or non-split based on whether there was enough data to be worth splitting into a .dwo file, but I guess that situation might still be producing an empty .dwo file which isn't ideal - I haven't tested that.

Thu, Jan 14, 10:20 AM · Restricted Project
MaskRay accepted D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..

Stops dumping default values for Version, Flags, VersionNdx and Hash fields.

Thu, Jan 14, 9:38 AM · Restricted Project
MaskRay added a comment to D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..

LSB 1.3 is superseded. The description can link to https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/symversion.html instead

Thu, Jan 14, 9:31 AM · Restricted Project
MaskRay accepted D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..

LGTM.

Thu, Jan 14, 9:18 AM · Restricted Project
MaskRay added a comment to D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input.

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

Thu, Jan 14, 9:10 AM · Restricted Project

Wed, Jan 13

MaskRay added reviewers for D94620: [NFC] Disallow unused prefixes under MC/ARM: dmgreen, samparker.
Wed, Jan 13, 11:44 PM · Restricted Project
MaskRay requested review of D94655: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input.
Wed, Jan 13, 11:39 PM · Restricted Project
MaskRay added inline comments to D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.
Wed, Jan 13, 11:03 PM · Restricted Project
MaskRay added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Wed, Jan 13, 9:53 PM · Restricted Project
MaskRay updated subscribers of D94560: [ELF] report section sizes when output file too large.
Wed, Jan 13, 9:03 PM · Restricted Project
MaskRay committed rG53b34601abf1: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index= (authored by MaskRay).
[Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=
Wed, Jan 13, 9:02 PM
MaskRay closed D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.
Wed, Jan 13, 9:02 PM · Restricted Project
MaskRay accepted D94649: [StringExtras] Rename SubsequentDelim to ListSeparator.

Thanks. The new name looks better! I mentioned the patch on discord. Hope one day or two is sufficient for interested folks to weigh in.

Wed, Jan 13, 9:01 PM · Restricted Project
MaskRay retitled D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index= from [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN if -fthinlto-index= is specified to [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.
Wed, Jan 13, 8:53 PM · Restricted Project
MaskRay updated the summary of D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.
Wed, Jan 13, 8:48 PM · Restricted Project
MaskRay added a comment to D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.

In implicit thinlto this seemed to work for me without any changes:

$ clang++-tot -flto=thin test.cpp -g -c
$ clang++-tot -flto=thin -fuse-ld=lld -gsplit-dwarf test.o && llvm-dwarfdump-tot a.out
a.out:  file format elf64-x86-64

.debug_info contents:
0x00000000: Compile Unit: length = 0x0000002c, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x00000030)

0x0000000b: DW_TAG_compile_unit
              DW_AT_stmt_list   (0x00000000)
              DW_AT_comp_dir    ("/usr/local/google/home/blaikie/dev/scratch")
              DW_AT_GNU_dwo_name        ("a.out_dwo/1.dwo")
              DW_AT_GNU_dwo_id  (0xf29563d7c812deae)
              DW_AT_low_pc      (0x0000000000201730)
              DW_AT_high_pc     (0x0000000000201738)
              DW_AT_GNU_addr_base       (0x00000000)

Any idea why that worked without the need for a special case/changes? Might be nice if both these use cases (implicit and explicit thinlto) somehow managed to use more common code generation options. But I guess that's too different - probably some generic "take all the clang flags and pass them through to the linker to pass back to the compiler", which isn't really relevant to this... well, maybe (if they're being passed back to the compiler in implicit thin lto, how does implicit thin lto work correctly without the -g flag?)

Wed, Jan 13, 8:31 PM · Restricted Project
MaskRay updated the diff for D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.

improve code comment

Wed, Jan 13, 7:40 PM · Restricted Project
MaskRay updated the summary of D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=.
Wed, Jan 13, 7:37 PM · Restricted Project