Page MenuHomePhabricator

scott.linder (Scott Linder)
User

Projects

User Details

User Since
Jan 15 2018, 8:31 AM (280 w, 2 d)

Recent Activity

Yesterday

scott.linder accepted D151806: [AsmPrinter][AMDGPU] Generate uwtable entries in .eh_frame.

LGTM; I always have to re-learn what the combinations of uwtable and nounwind mean, but this seems reasonable to me

Wed, May 31, 11:45 AM · Restricted Project, Restricted Project

Tue, May 23

scott.linder committed rG7f033d0f756e: [llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE (authored by scott.linder).
[llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE
Tue, May 23, 1:53 PM · Restricted Project, Restricted Project
scott.linder closed D150713: [llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE.
Tue, May 23, 1:52 PM · Restricted Project, Restricted Project
scott.linder added inline comments to D143463: [X86] Use the CFA when appropriate for better variable locations around calls..
Tue, May 23, 1:26 PM · Restricted Project, Restricted Project
scott.linder committed rG3be667ae5a10: [X86] Use the CFA when appropriate for better variable locations around calls. (authored by khuey).
[X86] Use the CFA when appropriate for better variable locations around calls.
Tue, May 23, 1:25 PM · Restricted Project, Restricted Project
scott.linder closed D143463: [X86] Use the CFA when appropriate for better variable locations around calls..
Tue, May 23, 1:25 PM · Restricted Project, Restricted Project

Mon, May 22

scott.linder accepted D143463: [X86] Use the CFA when appropriate for better variable locations around calls..

LGTM, thank you again for attempting the switch to doing this unconditionally! I was clearly wrong about there being no dependence if even an in-tree project is breaking

Mon, May 22, 4:00 PM · Restricted Project, Restricted Project

Wed, May 17

scott.linder added a comment to D150713: [llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE.

(Cross-posting from the other thread, to keep everything in one place)

Wed, May 17, 11:03 AM · Restricted Project, Restricted Project

Tue, May 16

scott.linder added a comment to D147270: [DebugInfo] Support more than 2 operands in DWARF operations.

I took a stab at a patch to address the assertion at D150713

Tue, May 16, 1:26 PM · Restricted Project, Restricted Project
scott.linder added reviewers for D150713: [llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE: CarlosAlbertoEnciso, echristo, dblaikie, probinson, psamolysov, Orlando.
Tue, May 16, 1:22 PM · Restricted Project, Restricted Project
scott.linder requested review of D150713: [llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE.
Tue, May 16, 1:20 PM · Restricted Project, Restricted Project
scott.linder added a comment to D147270: [DebugInfo] Support more than 2 operands in DWARF operations.

Is there a good target you have in mind for the benchmark? I seem to hit an assertion when dogfooding RelWithDebInfo binaries:

$ build/bin/llvm-debuginfo-analyzer --print=symbols build-relwithdebinfo/bin/llvm-tblgen
llvm-debuginfo-analyzer: /home/slinder1/llvm-project/main/llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h:102: virtual v
oid llvm::logicalview::LVSymbol::setReference(llvm::logicalview::LVElement *): Assertion `(!Element || isa<LVSymbol>(Element)) &
& "Invalid element"' failed.

I've also tried `llc`, `clang`, and `llvm-debuginfo-analyzer` itself, all hit the same assert. Is there something I might be doing wrong on my end?

Sorry for my delay in answering but I was at the LLVM Euro 2023. Your command line is correct.

At Sony, we have managed to reproduce the same issue with a debug/checking build on a private project.

Tue, May 16, 8:43 AM · Restricted Project, Restricted Project

Thu, May 11

scott.linder added inline comments to D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir.
Thu, May 11, 2:05 PM · Restricted Project, Restricted Project
scott.linder added inline comments to D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941.
Thu, May 11, 11:37 AM · Restricted Project, Restricted Project, Restricted Project
scott.linder added a comment to D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir.

Does GCC have the same -ftime-trace= option? It seems like it doesn't, as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92396 is still open?

Thu, May 11, 11:19 AM · Restricted Project, Restricted Project

Tue, May 9

scott.linder added a comment to D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking.

I guess my main question is: What's the motivation for implementing this? Do you have a need/use for this? (it doesn't seem to be motivated by GCC compatibility - as discussed, looks like we're diverging in a bunch of ways from their behavior and the argument made that these are "developer" flags, so not a stable/compatible interface used across both compilers)

Tue, May 9, 12:00 PM · Restricted Project, Restricted Project

Mon, May 8

scott.linder added a comment to D147270: [DebugInfo] Support more than 2 operands in DWARF operations.

@scott.linder: Sorry for my delay. As pointed out by @dblaikie, it would be interesting to see the memory usage, specially for the logical view. Thanks.

Mon, May 8, 2:05 PM · Restricted Project, Restricted Project

Apr 27 2023

scott.linder added a comment to D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking.

I am OK to give LGTM, assuming the other reviewers don't still have reservations?

Apr 27 2023, 11:56 AM · Restricted Project, Restricted Project
scott.linder added a comment to D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking.

I think the patch as-is implements all the useful parts of GCC's complex rules and in the absence of -dumpbase (we deliberately don't implement), the rule almost exactly matches GCC unless we do gcc -g -gsplit-dwarf d/a.c -o e/a.out (instead of other filenames).

https://maskray.me/blog/2023-04-25-compiler-output-files records all my findings.

Apr 27 2023, 9:07 AM · Restricted Project, Restricted Project

Apr 26 2023

scott.linder added inline comments to D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking.
Apr 26 2023, 3:08 PM · Restricted Project, Restricted Project

Apr 25 2023

scott.linder added a comment to D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking.

I'm a bit confused after trying to work out the rules for the GCC version of -dumpdir over at https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-dumpdir but it at least seems like our version is a subset of theirs.

Apr 25 2023, 3:27 PM · Restricted Project, Restricted Project
scott.linder accepted D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match.

LGTM, thank you!

Apr 25 2023, 2:04 PM · Restricted Project, Restricted Project, Restricted Project
scott.linder added inline comments to D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match.
Apr 25 2023, 10:58 AM · Restricted Project, Restricted Project, Restricted Project

Apr 24 2023

scott.linder added inline comments to D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match.
Apr 24 2023, 2:27 PM · Restricted Project, Restricted Project, Restricted Project

Apr 19 2023

scott.linder added a reviewer for D147270: [DebugInfo] Support more than 2 operands in DWARF operations: CarlosAlbertoEnciso.
Apr 19 2023, 4:23 PM · Restricted Project, Restricted Project
scott.linder updated the diff for D147270: [DebugInfo] Support more than 2 operands in DWARF operations.

Now that we are no longer contrained by constexpr, just use
SmallVector for everything.

Apr 19 2023, 4:21 PM · Restricted Project, Restricted Project
scott.linder abandoned D147267: [NFC][DebugInfo] Make Descriptions constexpr.

Determined constexpr was not useful here

Apr 19 2023, 1:52 PM · Restricted Project, Restricted Project
scott.linder committed rG7f596bb50944: [DebugInfo] printCompactDWARFExpr: don't assert on stack size (authored by scott.linder).
[DebugInfo] printCompactDWARFExpr: don't assert on stack size
Apr 19 2023, 1:49 PM · Restricted Project, Restricted Project
scott.linder closed D147269: [DebugInfo] printCompactDWARFExpr: don't assert on stack size.
Apr 19 2023, 1:49 PM · Restricted Project, Restricted Project
scott.linder committed rGa4fb7f60e26c: [HeterogeneousDWARF] Update encodings in… (authored by scott.linder).
[HeterogeneousDWARF] Update encodings in…
Apr 19 2023, 1:41 PM · Restricted Project, Restricted Project
scott.linder closed D147265: Update encodings in AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst.
Apr 19 2023, 1:41 PM · Restricted Project, Restricted Project
scott.linder removed reviewers for D147265: Update encodings in AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst: debug-info, dblaikie.

Oh, naming though - DW_OP_LLVM_extension, perhaps? But I don't mind too much - the name isn't load bearing anyway, we can always rename it if we find a more descriptive name idf we're preserving the semantics

Apr 19 2023, 1:31 PM · Restricted Project, Restricted Project
scott.linder committed rG974f973d26a6: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE (authored by scott.linder).
[dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE
Apr 19 2023, 8:02 AM · Restricted Project, Restricted Project
scott.linder closed D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.
Apr 19 2023, 8:02 AM · Restricted Project, Restricted Project

Apr 18 2023

scott.linder accepted D148607: Add support for writing binary ELF doc nodes.

LGTM with 1 small nit

Apr 18 2023, 11:59 AM · Restricted Project, Restricted Project

Apr 17 2023

scott.linder accepted D143463: [X86] Use the CFA when appropriate for better variable locations around calls..

LGTM, thank you!

Apr 17 2023, 1:02 PM · Restricted Project, Restricted Project

Apr 13 2023

scott.linder added a comment to D143463: [X86] Use the CFA when appropriate for better variable locations around calls..

I'm happy to update this to do 1 + 2 + 3 too, it's not hard.

Apr 13 2023, 12:46 PM · Restricted Project, Restricted Project
scott.linder accepted D143463: [X86] Use the CFA when appropriate for better variable locations around calls..

Very reasonable, but if GCC is already using the CFA unconditionally it seems likely that implies a pretty good support base?

Indeed.

I meant to propose another middle-ground, where we use the CFA whenever !hasFP, which (IIUC) is a precondition for x86 ever adjusting the CFA. It would mean we use the CFA more than with the heuristic, but still not unconditionally.

!hasFP is a precondition for x86 emitting CFA offset adjustment directives solely because if there is a frame pointer LLVM simply defines the CFA to be the frame pointer. And then the frame pointer remains the same regardless of adjustments to %rsp, so the CFA doesn't need to be adjusted simultaneously. See the createDefCfaRegister call inside X86FrameLowering::emitPrologue (which is gated on, at the top level, HasFP).

I think I am mostly concerned with the statefulness of the heuristic, coupled with the fact that it seems to not be completely contained. The majority of it is handled by X86FrameLowering, but there is at least the one instance where it has to be explicitly managed in X86MCInstLower. If others are not as concerned with this I'm happy to concede on it, but it is the main concern I have.

The not-being-contained is just because X86::MOVPC32r is really weird.

OK, I may have just been overly concerned about this; my familiarity with the X86 target in LLVM is very limited.

I definitely understand the complexity in DwarfExpression::addMachineRegExpression makes any change very difficult. I am happy with continuing to apply the offset to the CFA in the frame base, and if some other brave soul wants to change that in a future patch they won't be any worse off.

Yeah I don't really want to do that here but I agree that folding the offset between the CFA and the stack pointer into the variable expressions rather than leaving it in the DW_AT_frame_base expression is the ideal end point.

My current leaning is to change to using the CFA unconditionally, but offset to maintain the same frame base value. It will result in a slightly larger expression in the default case, but I would be much more confident that the result is always correct.

So, to be clear, you're learning towards using a CFA-based DW_AT_frame_base in all three of these scenarios?

  1. There is a CFA, and LLVM is currently using %rsp as the DW_AT_frame_base, and %rsp changes throughout the function.
  2. There is a CFA, and LLVM is currently using %rsp as the DW_AT_frame_base, and %rsp remains constant (after the prologue).
  3. There is a CFA, and LLVM is currently using %rbp as the DW_AT_frame_base.

Where using the CFA for 1 is what this patch as originally proposed does, and 1 + 2 is the "middle ground" you suggested previously, as I understand it.

I think using the CFA for 1 + 2 + 3 makes more sense than doing just 1 + 2, so I think we more or less agree here.

Apr 13 2023, 11:27 AM · Restricted Project, Restricted Project

Apr 12 2023

scott.linder updated subscribers of D143463: [X86] Use the CFA when appropriate for better variable locations around calls..

As a bit of a side-note, I think X86 always maintains a "precise CFA" to refer to, but that may not always be true (see https://reviews.llvm.org/D14948).

This is relying on the CFA being correct when it's present. CFI is not always present (see X86FrameLowering::needsDwarfCFI which at a minimum excludes some Win64 stuff) but it might be present in all cases that matter. I'm not sure.

I'm curious about a couple points:

  • If we do always have the CFA available, then is there any reason not to use it unconditionally?

In decreasing order of persuasiveness to me

  1. If the frame pointer is present (or the stack pointer remains unchanged) using it is simpler for the debug info consumer (as there's no need to consult the CFI and run through its state machine). In theory there might also be old tools out there that don't support CFI as well.

Very reasonable, but if GCC is already using the CFA unconditionally it seems likely that implies a pretty good support base?

  1. If the frame pointer is present (or the stack pointer remains unchanged) using it works today. Don't fix what isn't broke.

I definitely think this holds a lot of weight, but just having one concept (CFA) instead of several (stack-pointer, frame-pointer, frame-base, CFA) is also very attractive. The fact that the patch is even necessary also demonstrates something is broken, although it might be the exception that proves the rule.

  1. If there are any bugs out there that result in an incorrect CFA the machine registers are more likely to be correct as they're actually used by the program and thus issues are more likely to be noticed.

If the CFA is incorrect, unwinding seems like it is doomed to fail, which should constitute a program execution bug if e.g. exceptions are enabled, right? I don't necessarily think we will end up with more elusive debug-info bugs as a result of relying on the CFA.

Although it seems the gcc maintainers were not persuaded by any of this. gcc emits DW_OP_call_frame_cfa in every circumstance I can think of (at least on x86-64).

  • If we cannot use it unconditionally, can the condition be more direct, i.e. can we remove the heuristic?

I'm not aware of any reason we cannot use the CFA unconditionally (at least on x86) if it is present. I believe it is possible to remove the setHasCFIAdjustCfa heuristic and use the CFA in place of the stack pointer unconditionally if the above arguments are not persuasive. I believe it's also possible to use the CFA in place of the frame pointer even when it's present, if desired.

I meant to propose another middle-ground, where we use the CFA whenever !hasFP, which (IIUC) is a precondition for x86 ever adjusting the CFA. It would mean we use the CFA more than with the heuristic, but still not unconditionally.

Apr 12 2023, 1:00 PM · Restricted Project, Restricted Project

Apr 11 2023

scott.linder added a reviewer for D143463: [X86] Use the CFA when appropriate for better variable locations around calls.: scott.linder.
Apr 11 2023, 2:52 PM · Restricted Project, Restricted Project
scott.linder added a comment to D143463: [X86] Use the CFA when appropriate for better variable locations around calls..

Thank you for the patch! Sorry for the long delay in review

Apr 11 2023, 2:51 PM · Restricted Project, Restricted Project

Apr 6 2023

scott.linder added a comment to D147267: [NFC][DebugInfo] Make Descriptions constexpr.

Sure - though could you check if there's any revision history that explains the comment further/motivates why this would be valuable? (I have some apprehension about making extra things constexpr as it can increase compile time excessively (forcing the frontend to check for evaluation - when the backend can more cheaply optimize it away later anyway))

Sure, I did just blindly assume the TODO had merit. Is there a structured way you tend to evaluate these on a case-by-case basis? Should I compare instructions retired while compiling LLVM+Clang and while using the resulting LLVM+Clang to inspect DebugInfo with and without the change?

Edit:

I compared building the lib/DebugInfo/DWARF/all target without the patch vs. with the patch, with some minimal isolation and attempts to make the result statistically significant without spending too much time:

<stat>: <control> <constexpr> (<constexpr>/<control>)
lib/libLLVMDebugInfoDWARF.a-size: 2021132 2016068 (0.997494)
instructions:u: 4271564407084 4271164683246 (0.999906)
Maximum resident set size: 694690 694689 (0.999999)
branches: 495120306752 495185537556 (1.000132)
branch-misses: 30440381987 30515429111 (1.002465)
User time: 2321.876000 2331.440000 (1.004119)

If https://www.npopov.com/2020/05/10/Make-LLVM-fast-again.html is to be believed the user instructions retired and max-rss metrics are the best proxies available, but I included some of the "noisy" metrics too. None of them seem too outrageously out-of-line, but I can rerun to see if e.g. the 0.4% increase user-time is a fluke.

I stole the cmake, time, and perf command-lines from the repo for http://llvm-compile-time-tracker.com/ referenced from the same blog post above. I haven't done much in this area so I don't know if I made any mistakes; my script is:

#!/bin/bash

build() {
  for f in control constexpr; do
    rm -rf build-$f
    mkdir -p build-$f
    rm -rf out-$f
    mkdir -p out-$f
    cmake \
      -GNinja \
      -S./source-$f/llvm \
      -B./build-$f \
      -DLLVM_ENABLE_PROJECTS="clang" \
      -DLLVM_TARGETS_TO_BUILD="X86" \
      -DLLVM_BUILD_TOOLS=false \
      -DLLVM_APPEND_VC_REV=false \
      -DCMAKE_BUILD_TYPE=Release \
      -DLLVM_CCACHE_BUILD=false \
      -DLLVM_USE_LINKER=gold \
      -DLLVM_BINUTILS_INCDIR=/usr/include \
      -DCLANG_ENABLE_ARCMT=false \
      -DCLANG_ENABLE_STATIC_ANALYZER=false
    /usr/bin/ninja -C build-$f lib/DebugInfo/DWARF/all
    for i in $(seq 5); do
      LC_ALL=C ninja -C build-$f clean
      LC_ALL=C \
        /usr/bin/time -v -o out-$f/time.$i \
        perf stat -x \; -o out-$f/perf.$i \
          -e instructions \
          -e instructions:u \
          -e cycles \
          -e task-clock \
          -e branches \
          -e branch-misses \
          ninja -C build-$f lib/DebugInfo/DWARF/all
    done
  done
}
print_stat_header() {
  printf "<stat>: <control> <constexpr> (<constexpr>/<control>)\n"
}
print_stat() {
  printf "%s: %s %s (%s)\n" "$1" "$2" "$3" "$(awk -- "BEGIN { printf \"%f\\n\", $3 / $2 }")"
}
size() {
  stat --printf='%s\n' "$@"
}
compare_size() {
  local file="$1"
  local control="$(size build-control/$file)"
  local constexpr="$(size build-constexpr/$file)"
  print_stat "$file-size" "$control" "$constexpr"
}
compare() {
  local dataset="$1"; shift
  local rowname="$1"; shift
  local fmt="$1"; shift
  local sep=:
  local idx=2
  if [[ $dataset == perf ]]; then
    sep=\;
    idx=1
  fi
  local awkprogram="/$rowname/ { sum += \$$idx; n++; } END { printf \"$fmt\\n\", sum / n; }"
  local control=$(awk -F "$sep" -- "$awkprogram" out-control/$dataset.*)
  local constexpr=$(awk -F "$sep" -- "$awkprogram" out-constexpr/$dataset.*)
  local relative=$(awk -- "BEGIN { printf \"%f\\n\", $constexpr / $control }")
  print_stat "$rowname" "$control" "$constexpr"
}
print() {
  print_stat_header
  compare_size lib/libLLVMDebugInfoDWARF.a
  compare perf 'instructions:u' '%u'
  compare time 'Maximum resident set size' '%u'
  compare perf 'branches' '%u'
  compare perf 'branch-misses' '%u'
  compare time 'User time' '%f'
}
main() {
  build
  print
}
"$@"

Run as bench.sh main

I will expand this to building the whole of llvm, and then dogfood the result, although I may not run as many trials to avoid it taking too long.

Apr 6 2023, 3:43 PM · Restricted Project, Restricted Project

Apr 5 2023

scott.linder added a comment to D147270: [DebugInfo] Support more than 2 operands in DWARF operations.

Got any measurements of memory usage? (seems like this'd increase memory usage? Maybe try a self-host release build (optimizations causing more interesting locations and so more location operations) & see how peak memory usage in various compilation changes?)

Apr 5 2023, 1:56 PM · Restricted Project, Restricted Project
scott.linder added a comment to D147269: [DebugInfo] printCompactDWARFExpr: don't assert on stack size.

Not the best error handling (like might be better returning Error from this function?) but probably OK.

Apr 5 2023, 1:53 PM · Restricted Project, Restricted Project
scott.linder added inline comments to D147271: [DebugInfo] Add DW_OP_LLVM_user extension point.
Apr 5 2023, 1:50 PM · Restricted Project, Restricted Project
scott.linder added a comment to D147267: [NFC][DebugInfo] Make Descriptions constexpr.

Sure - though could you check if there's any revision history that explains the comment further/motivates why this would be valuable? (I have some apprehension about making extra things constexpr as it can increase compile time excessively (forcing the frontend to check for evaluation - when the backend can more cheaply optimize it away later anyway))

Apr 5 2023, 11:06 AM · Restricted Project, Restricted Project

Mar 31 2023

scott.linder updated the summary of D147279: [HeterogeneousDWARF] Implement AMDGPU CFI, DebugInfo.
Mar 31 2023, 7:13 AM · Restricted Project, Restricted Project, Restricted Project
scott.linder added a comment to D147271: [DebugInfo] Add DW_OP_LLVM_user extension point.

I've also submitted a proposal for a standard version of the operation (thread at https://lists.dwarfstd.org/pipermail/dwarf-discuss/2023-March/002199.html)

Mar 31 2023, 7:10 AM · Restricted Project, Restricted Project

Mar 30 2023

scott.linder added a reviewer for D147279: [HeterogeneousDWARF] Implement AMDGPU CFI, DebugInfo: debug-info.
Mar 30 2023, 3:44 PM · Restricted Project, Restricted Project, Restricted Project
scott.linder added a reviewer for D147278: [HeterogeneousDWARF] Implement extensions in DebugInfo: debug-info.
Mar 30 2023, 3:44 PM · Restricted Project, Restricted Project
scott.linder added a reviewer for D147271: [DebugInfo] Add DW_OP_LLVM_user extension point: debug-info.
Mar 30 2023, 3:44 PM · Restricted Project, Restricted Project
scott.linder added a reviewer for D147270: [DebugInfo] Support more than 2 operands in DWARF operations: debug-info.
Mar 30 2023, 3:43 PM · Restricted Project, Restricted Project
scott.linder added a reviewer for D147269: [DebugInfo] printCompactDWARFExpr: don't assert on stack size: debug-info.
Mar 30 2023, 3:43 PM · Restricted Project, Restricted Project
scott.linder added a reviewer for D147267: [NFC][DebugInfo] Make Descriptions constexpr: debug-info.
Mar 30 2023, 3:43 PM · Restricted Project, Restricted Project
scott.linder added a member for debug-info: scott.linder.
Mar 30 2023, 3:43 PM
scott.linder added a reviewer for D147265: Update encodings in AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst: debug-info.
Mar 30 2023, 3:42 PM · Restricted Project, Restricted Project
scott.linder abandoned D76880: [AMDGPU] Emit entry function CFI.

Abandoned in favor of D147279 which avoids using most/all of the remaining user opcode

Mar 30 2023, 3:41 PM · Restricted Project, debug-info, Restricted Project
scott.linder abandoned D76882: [AMDGPU] Implement CFI for non-kernel functions.

Abandoned in favor of D147279 which avoids using most/all of the remaining user opcode

Mar 30 2023, 3:40 PM · Restricted Project, debug-info, Restricted Project
scott.linder abandoned D76878: Implement DW_{OP,AT}_LLVM_* for Heterogeneous Debugging.

Abandoned in favor of D147279 which avoids using most/all of the remaining user opcode

Mar 30 2023, 3:39 PM · Restricted Project, debug-info, Restricted Project
scott.linder abandoned D103223: [ADT][WIP] Proof of concept impl of generic visit for PointerUnion.
Mar 30 2023, 3:20 PM · Restricted Project, Restricted Project
scott.linder updated the diff for D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

Rebase

Mar 30 2023, 3:01 PM · Restricted Project, Restricted Project
scott.linder requested review of D147279: [HeterogeneousDWARF] Implement AMDGPU CFI, DebugInfo.
Mar 30 2023, 2:59 PM · Restricted Project, Restricted Project, Restricted Project
scott.linder requested review of D147278: [HeterogeneousDWARF] Implement extensions in DebugInfo.
Mar 30 2023, 2:58 PM · Restricted Project, Restricted Project
scott.linder requested review of D147271: [DebugInfo] Add DW_OP_LLVM_user extension point.
Mar 30 2023, 1:53 PM · Restricted Project, Restricted Project
scott.linder requested review of D147270: [DebugInfo] Support more than 2 operands in DWARF operations.
Mar 30 2023, 1:52 PM · Restricted Project, Restricted Project
scott.linder requested review of D147269: [DebugInfo] printCompactDWARFExpr: don't assert on stack size.
Mar 30 2023, 1:51 PM · Restricted Project, Restricted Project
scott.linder requested review of D147267: [NFC][DebugInfo] Make Descriptions constexpr.
Mar 30 2023, 1:48 PM · Restricted Project, Restricted Project
scott.linder requested review of D147265: Update encodings in AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst.
Mar 30 2023, 1:46 PM · Restricted Project, Restricted Project

Mar 21 2023

scott.linder added inline comments to D146031: [AMDGPU] Add MMOs for GFX11 Streamout Instructions.
Mar 21 2023, 8:44 AM · Restricted Project, Restricted Project

Mar 20 2023

scott.linder added inline comments to D146031: [AMDGPU] Add MMOs for GFX11 Streamout Instructions.
Mar 20 2023, 2:15 PM · Restricted Project, Restricted Project
scott.linder accepted D146232: [NFC] Add iterator traits to BitVector set_bits_iterator.
Mar 20 2023, 12:29 PM · Restricted Project, Restricted Project
scott.linder accepted D145558: [Assignment Tracking][NFC] Use BitVectors as masks for SmallVectors rather than using DenseMaps.

Thank you @scott.linder for taking a look at this.

It might be worth including the rationale in the description/commit message, like you've done in other similar changes. I assume it is a performance improvement?

Yep there's an average of around 0.25% reduction in instructions retired and a 1.1% max-rss for the CTMark suite in LTO-O3-g builds (I'll update the description).

I'm leaving a bunch of edit suggestions, but feel free to ignore them! I was just going through the exercise of writing one possible implementation to prove to myself it is an actual improvement, YMMV

I appreciate the in-depth review and the suggestions all look good to me!

The diff has become quite large now so I'll could look at trying to factor out some of the suggestions into NFC patches?

Mar 20 2023, 12:10 PM · Restricted Project, Restricted Project, debug-info

Mar 15 2023

scott.linder added a comment to D145558: [Assignment Tracking][NFC] Use BitVectors as masks for SmallVectors rather than using DenseMaps.

It might be worth including the rationale in the description/commit message, like you've done in other similar changes. I assume it is a performance improvement?

Mar 15 2023, 10:47 AM · Restricted Project, Restricted Project, debug-info

Mar 14 2023

scott.linder added inline comments to D145515: [Assignment Tracking][NFC] Only calculate fragment overlaps for partially stack homed variables.
Mar 14 2023, 3:57 PM · Restricted Project, Restricted Project
scott.linder added inline comments to D145512: [Assignment Tracking][NFC] Reuse BlockInfo rather than re-allocating each iteration.
Mar 14 2023, 3:55 PM · debug-info, Restricted Project, Restricted Project

Mar 13 2023

scott.linder added a comment to D145770: [clang-offload-bundler] Standardize TargetID field for bundler.

Is there a corresponding document update to go along with this? If consumers of the bundles will begin to rely on this it should be documented that it is guaranteed by the producer.

Mar 13 2023, 2:57 PM · Restricted Project, Restricted Project

Mar 6 2023

scott.linder updated the diff for D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

Address feedback:

  • Fix test name to reflect it testing both r600 and amdcn
  • Common up some checks between r600 and amdgcn
  • Adjust some whitespace to make test more readable
Mar 6 2023, 12:34 PM · Restricted Project, Restricted Project
scott.linder added inline comments to D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.
Mar 6 2023, 12:25 PM · Restricted Project, Restricted Project

Mar 1 2023

scott.linder updated the diff for D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

Address feedback

Mar 1 2023, 1:39 PM · Restricted Project, Restricted Project
scott.linder added a comment to D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

It looks like this code is testing code in libObject, but the testing is in llvm-dwarfdump. Should the tests be moved to test/Object?

Mar 1 2023, 1:38 PM · Restricted Project, Restricted Project

Feb 27 2023

scott.linder added a comment to D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

Ping

Feb 27 2023, 10:16 AM · Restricted Project, Restricted Project

Feb 21 2023

scott.linder accepted D144486: [Assignment Tracking] Initialise maps with exact required number of entries.
Feb 21 2023, 10:28 AM · Restricted Project, Restricted Project
scott.linder accepted D144483: [Assignment Tracking] Choose better passes for RemoveRedundantDbgInstrs call.

Seems reasonable to me! LGTM with a small nit

Feb 21 2023, 10:25 AM · Restricted Project, Restricted Project, debug-info
scott.linder accepted D144481: [Assignment Tracking] Only set module flag if pass modifies a function.
Feb 21 2023, 8:55 AM · Restricted Project, debug-info, Restricted Project

Feb 20 2023

scott.linder updated the diff for D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

Address feedback

Feb 20 2023, 10:44 AM · Restricted Project, Restricted Project
scott.linder added a comment to D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.

The patch title and code are to do with llvm-dwarfdump, but the commit message refers to llvm-objdump. Is this just a mistake?

Feb 20 2023, 10:19 AM · Restricted Project, Restricted Project

Feb 17 2023

scott.linder added reviewers for D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE: kzhuravl, arsenm, t-tye.
Feb 17 2023, 2:16 PM · Restricted Project, Restricted Project
scott.linder requested review of D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE.
Feb 17 2023, 2:16 PM · Restricted Project, Restricted Project

Feb 16 2023

scott.linder added a comment to D144176: [AMDGPU] Add cross-project-tests for WMMA builtins.

This is great, I also didn't realize there were such a thing :)

Feb 16 2023, 12:28 PM · Restricted Project, Restricted Project

Feb 15 2023

scott.linder committed rG97ba3c2bec48: [Clang][AMDGPU] Set LTO CG opt level based on Clang option (authored by scott.linder).
[Clang][AMDGPU] Set LTO CG opt level based on Clang option
Feb 15 2023, 9:35 AM · Restricted Project, Restricted Project
scott.linder committed rG45ee0a9afc62: [LLD] Add --lto-CGO[0-3] option (authored by scott.linder).
[LLD] Add --lto-CGO[0-3] option
Feb 15 2023, 9:35 AM · Restricted Project
scott.linder closed D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option.
Feb 15 2023, 9:34 AM · Restricted Project, Restricted Project
scott.linder closed D141970: [LLD] Add --lto-CGO[0-3] option.
Feb 15 2023, 9:34 AM · Restricted Project, Restricted Project, Restricted Project

Feb 9 2023

scott.linder accepted D137118: [YAML] Trim trailing whitespace from plain scalars.

Sorry for the delay in reviewing! LGTM with a couple nits

Feb 9 2023, 3:17 PM · Restricted Project, Restricted Project
scott.linder accepted D142453: [AMDGPU][MC] Generate relative relocations for allocatable (more particularly, eh_frame) sections.
Feb 9 2023, 10:08 AM · Restricted Project, Restricted Project

Feb 2 2023

scott.linder added a comment to D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option.

Are there any thoughts on whether this is too ugly to live? It will be awkward to teach users the current default behavior without this change, but if we can accept it as a historical quirk that may be OK.

Feb 2 2023, 2:48 PM · Restricted Project, Restricted Project
scott.linder updated the diff for D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option.

Rebase

Feb 2 2023, 2:41 PM · Restricted Project, Restricted Project
scott.linder updated the diff for D141970: [LLD] Add --lto-CGO[0-3] option.
  • Drop [Clang] from commit message.
  • ltocgo -> ltoCgo
Feb 2 2023, 2:41 PM · Restricted Project, Restricted Project, Restricted Project

Feb 1 2023

scott.linder updated the diff for D141970: [LLD] Add --lto-CGO[0-3] option.

Replace asserts added in previous version of patch with true diagnostic errors.
The only excpetion is COFF, where the parsing still deals in integral values
and ensures they fall in the right range, which the Driver assumes/asserts.

Feb 1 2023, 3:30 PM · Restricted Project, Restricted Project, Restricted Project
scott.linder added inline comments to D141970: [LLD] Add --lto-CGO[0-3] option.
Feb 1 2023, 9:59 AM · Restricted Project, Restricted Project, Restricted Project