Page MenuHomePhabricator

scott.linder (Scott Linder)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 15 2018, 8:31 AM (139 w, 5 d)

Recent Activity

Aug 19 2020

scott.linder accepted D86259: [AMDGPU] Correct DWARF register defintions.
Aug 19 2020, 6:18 PM · Restricted Project

Aug 17 2020

scott.linder accepted D84519: [llvm-objdump][AMDGPU] Detect CPU string.

LGTM, and can you please open a follow-up review once this is committed to hide all of the Object::get<Target><Stuff> methods?

Aug 17 2020, 10:18 AM · Restricted Project

Aug 13 2020

scott.linder added a comment to D82363: [DebugInfo] Add new instruction and expression operator for variadic debug values.

I don't know that my opinion holds much weight, but I agree that reducing the number of nearly-equivalent-but-subtly-different intrinsics/pseudo-instructions for debug-information would make it easier for a new dev to understand their semantics, and make it much easier to extend. I would argue that using different representations based on complexity doesn't make the result any more comprehensible, it just adds more things to comprehend.

Aug 13 2020, 1:31 PM · Restricted Project, debug-info
scott.linder added inline comments to D85882: [AMDGPU] Update subtarget features for new target ID support.
Aug 13 2020, 10:59 AM · Restricted Project
scott.linder added inline comments to D84519: [llvm-objdump][AMDGPU] Detect CPU string.
Aug 13 2020, 9:03 AM · Restricted Project

Aug 12 2020

scott.linder added a comment to D84519: [llvm-objdump][AMDGPU] Detect CPU string.

Looking good to me! Some small nits, and it would be good to hear back on what the general consensus on adding this to Object is.

Aug 12 2020, 12:30 PM · Restricted Project

Aug 7 2020

scott.linder added inline comments to D84519: [llvm-objdump][AMDGPU] Detect CPU string.
Aug 7 2020, 2:04 PM · Restricted Project

Aug 6 2020

scott.linder added a comment to D84519: [llvm-objdump][AMDGPU] Detect CPU string.

This seems to be related to the push of the cpu-id (aka. target-id) into other places as well: e.g., D84822, D80750, probably more.
I mentioned in those reviews already, this doesn't strike me as an "AMDGPU" feature at all.
We should start the discussions around missing functionality instead of adding AMDGPU workarounds in all these places...
One example is an IR module level target cpu and/or target feature, similar to target triple.


Also, the commit doesn't have a message that explains what is going on.

Aug 6 2020, 2:04 PM · Restricted Project

Aug 5 2020

scott.linder accepted D81364: AMDGPU: Correct prolog SP initialization logic.

LGTM, thank you!

Aug 5 2020, 12:41 PM · Restricted Project
scott.linder added a comment to D84519: [llvm-objdump][AMDGPU] Detect CPU string.

I am a bit worrisome that this patch is adding more target specific stuff to llvm-objdump.

Agreed, I have looked at making the majority of llvm-objdump (and essentially everything under tools/) into a library at one point, but the biggest initial hurdle is the CommandLine parsing library. I think ideally all of the target-relevant parts would be in target-overridden implementations in some library, all of the llvm-objdump behavior parts (like the core symbol-to-symbol loop) would be in an llvm-objdump library which depends on the libraries with target-specific behaviors, and the tools/ directory would contain ~10 line main functions.

Can we step back and think whether precise CPU selection is really necessary? For example, PowerPC has the needs for different ISA levels but it simply uses -mcpu=future.

I am a bit lost on what you mean; is there a significant cost to providing precise CPU selection? It doesn't make sense to require the user to e.g. run the dumper to inspect the binary in order to come up with the command-line to run the dumper in order to inspect the binary.

If different AMDGPU CPUs are incompatible, should such detection be moved to somewhere in lib/Target/AMDGPU/?

Yes, I think it should be done in a generic way. All other targets which don't need/want it just getting the default (existing) behavior.

I'd like to point out that this patch correctly detects the CPU string by looking at the binary. The CPU string needs to be determined beforehand and passed on to the SubtargetInfo constructor.

To move the CPU string detection to lib/Target/AMDGPU, the target would also need to look at the binary. I think, one would have to pass ObjectFile * to the MCSubtargetInfo constructor. This allows targets to do the needful in their respective constructors. I'd like to know thoughts/opinions on this.

Aug 5 2020, 10:03 AM · Restricted Project
scott.linder added inline comments to D81364: AMDGPU: Correct prolog SP initialization logic.
Aug 5 2020, 8:36 AM · Restricted Project

Aug 4 2020

scott.linder added a comment to D81070: [AMDGPU] Fixup use of StackPtrOffsetReg when not initialized.

The other option is to treat any frame that can possibly have an offset that won't fit in the immediate field as needing an SP

Aug 4 2020, 2:42 PM · Restricted Project
scott.linder added a comment to D81364: AMDGPU: Correct prolog SP initialization logic.

I don't know if this has anything to do with David's issue, but I think separately this makes sense. I'm just confused what the semantic difference between the two "do we need an SP" predicates is?

Aug 4 2020, 2:39 PM · Restricted Project

Jul 31 2020

scott.linder added a comment to D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors.

I'm probably not in a position to review the majority of this further. However, I do have big reservations about the testing - there are a high number of possible code paths, but I only see 5 tests, which clearly can't cover all these code paths.

Jul 31 2020, 9:36 AM · Restricted Project

Jul 30 2020

scott.linder accepted D60146: AMDGPU: Don't use generation to determine encoding.

Sorry for the insane delay, this makes perfect sense to me.

Jul 30 2020, 9:49 AM
scott.linder accepted D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors.

LGTM, thank you!

Jul 30 2020, 9:40 AM · Restricted Project

Jul 29 2020

scott.linder accepted D70523: [AMDGPU] Update AMDGPUUsage with DWARF proposal.
Jul 29 2020, 10:05 PM · debug-info, Restricted Project
scott.linder accepted D70523: [AMDGPU] Update AMDGPUUsage with DWARF proposal.
Jul 29 2020, 6:53 PM · debug-info, Restricted Project

Jul 28 2020

scott.linder added a comment to D84519: [llvm-objdump][AMDGPU] Detect CPU string.

Can we step back and think whether precise CPU selection is really necessary? For example, PowerPC has the needs for different ISA levels but it simply uses -mcpu=future.

I am a bit lost on what you mean; is there a significant cost to providing precise CPU selection? It doesn't make sense to require the user to e.g. run the dumper to inspect the binary in order to come up with the command-line to run the dumper in order to inspect the binary.

-mcpu=pwr10 decodes more instructions than -mcpu=pwr9.
-mcpu=pwr9 decodes more instructions than -mcpu=pwr8.

I think GNU objdump just decodes all instructions it recognizes, not expecting a -mcpu option. It behaves as if the default recognized ISA level includes everything.

Jul 28 2020, 8:12 AM · Restricted Project

Jul 27 2020

scott.linder added a comment to D84519: [llvm-objdump][AMDGPU] Detect CPU string.

I am a bit worrisome that this patch is adding more target specific stuff to llvm-objdump.

Jul 27 2020, 1:14 PM · Restricted Project
scott.linder added a comment to D84194: [AMDGPU] Correct the number of SGPR blocks used for GFX9.

I discussed with Tony today, and I was thinking about this the wrong way.

SPI does not require the granule count to be even, it just rounds up the granule count before actually performing the allocation. This means, from the compiler's perspective, when it is calculating things like the AMDGPU::IsaInfo::getMaxNumSGPRs it must consider the "allocation" granule size (IsaInfo::getSGPRAllocGranule). Conversely, from the assembler/diassembler perspective, it must consider the "encoding" granule size (IsaInfo::getSGPREncodingGranule). It is perfectly OK to have a GFX9 code object with a granulated SGPR count of 1, and we should allow emitting that in the assembler so that the disassembler can accurately reproduce those code objects.

I don't think there is any fix needed here, we already separate these two concepts and correctly apply them elsewhere. I think I just led you astray in the disassembly patch; you should only be using the encoding granule size, and shouldn't need any special handling for e.g. GFX9 to handle the fact that the allocation and encoding granule sizes are not equal.

Correct me if I'm wrong. So we must not take inverse of the mentioned GFX9 calculation (the one where we divide by 16 before roundup) as it is for allocation granule size? And hence the disassembly computation will be same for GFX6-8 and GFX9 (because the encoding granule size is the same)?

Jul 27 2020, 12:50 PM · Restricted Project

Jul 24 2020

scott.linder added inline comments to D84519: [llvm-objdump][AMDGPU] Detect CPU string.
Jul 24 2020, 8:07 AM · Restricted Project

Jul 23 2020

scott.linder added a comment to D84194: [AMDGPU] Correct the number of SGPR blocks used for GFX9.

I discussed with Tony today, and I was thinking about this the wrong way.

Jul 23 2020, 3:50 PM · Restricted Project
scott.linder added a comment to D84391: [AMDGPU] Fix incorrect arch assert while setting up FlatScratchInit.

Can we just add a predicate for whatever necessitates the SETREG for gfx10+? No idea on a good name, but if it were flatScratchIsHwreg() and it implied flatScratchIsPointer() this becomes:

if (ST.flatScratchIsHwreg()) {
  // pointer add then S_SETREG
} else if (ST.flatScratchIsPointer()) (
  // pointer add
} else {
  // size and offset dance
}
llvm_unreachable();

I agree that the existing assert here doesn't really mean anything, not sure why I wrote it.

Jul 23 2020, 2:31 PM · Restricted Project
scott.linder added a comment to D84391: [AMDGPU] Fix incorrect arch assert while setting up FlatScratchInit.

Can we just add a predicate for whatever necessitates the SETREG for gfx10+? No idea on a good name, but if it were flatScratchIsHwreg() and it implied flatScratchIsPointer() this becomes:

Jul 23 2020, 2:22 PM · Restricted Project
scott.linder added a comment to D80249: CodeGen: Don't lazily construct MachineFunctionInfo.

Do you really need to change the signature of MachineFunctionInfo::create? That seems orthogonal to the problem you're trying to solve, or perhaps a redundant solution.

The problem is: due to lazy construction and being able to refer to the MachineFunction, the state of the MachineFunctionInfo can have surprising accidental differences depending on when exactly it was created. There are two possible solutions:

  1. Prevent the MachineFunctionInfo from accessing the MachineFunction.
  2. Always create the MachineFunctionInfo in the same place.

This change does both, but either solution should be sufficient, and I personally find #1 counter-intuitive: it seems to me that the additional target-specific info about an object should be allowed to access that object.

You can't really do anything with the empty MachineFunction on construction. At the construction point, there isn't any information to access and it would always be a mistake to inspect it or make any construction decisions based on it (it would also be bad for the constructor to modify the MachineFunction).

MFI is mutable, and so referring back to the MachineFunction as part of the mutation is potentially something reasonable to do. So the point isn't to use data from the MachineFunction during construction of the MFI, but rather to allow the "child" object to have a backlink to its "parent".

Jul 23 2020, 2:09 PM · Restricted Project

Jul 21 2020

scott.linder requested changes to D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors.
Jul 21 2020, 3:28 PM · Restricted Project
scott.linder added inline comments to D84194: [AMDGPU] Correct the number of SGPR blocks used for GFX9.
Jul 21 2020, 3:08 PM · Restricted Project
scott.linder added a comment to D84194: [AMDGPU] Correct the number of SGPR blocks used for GFX9.

Needs test

I think these changes are tested using the test in https://reviews.llvm.org/D80713.
In fact the issue was found when testing round tripping for the above patch. I guess this can only be verified by a round trip test when we assemble->disassemble->re-assemble? Such a test is already present in the patch for D80713.

I am not sure how else can we look at the value of GRANULATED_WAVEFRONT_SGPR_COUNT in a test case.

Jul 21 2020, 2:47 PM · Restricted Project

Jul 16 2020

scott.linder added a comment to D80249: CodeGen: Don't lazily construct MachineFunctionInfo.

Do you really need to change the signature of MachineFunctionInfo::create? That seems orthogonal to the problem you're trying to solve, or perhaps a redundant solution.

The problem is: due to lazy construction and being able to refer to the MachineFunction, the state of the MachineFunctionInfo can have surprising accidental differences depending on when exactly it was created. There are two possible solutions:

  1. Prevent the MachineFunctionInfo from accessing the MachineFunction.
  2. Always create the MachineFunctionInfo in the same place.

This change does both, but either solution should be sufficient, and I personally find #1 counter-intuitive: it seems to me that the additional target-specific info about an object should be allowed to access that object.

Jul 16 2020, 1:19 PM · Restricted Project

Jul 9 2020

scott.linder added inline comments to D82818: AMDGPU: Remove .value_type from kernel metadata.
Jul 9 2020, 3:25 PM · Restricted Project

Jul 8 2020

scott.linder added a comment to D82858: [llvm-objdump] Detect note section for ELF objects.

In an ideal world, we'd merge all the binary tools (GNU and LLVM) into a single tool, or redistribute functionality somehow, so that we don't have duplicate functionality like we already do. This takes us further away from that ideal.

Jul 8 2020, 11:02 AM · Restricted Project

Jul 6 2020

scott.linder added inline comments to D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors.
Jul 6 2020, 1:06 PM · Restricted Project

Jul 1 2020

scott.linder added a comment to D81364: AMDGPU: Correct prolog SP initialization logic.

Sorry for the delay in reviewing, I've tried to wrap my head around this and I'm still lost.

Jul 1 2020, 2:36 PM · Restricted Project
scott.linder added a comment to D80713: [AMDGPU] Support disassembly for AMDGPU kernel descriptors.

No idea about the functional logic of this code, but I do wonder whether you'd be better off dividing it into smaller patches for testability.

This patch is for entirely disassembling the kernel descriptor symbol. It happens to be so that many directives affect the kernel descriptor. I will also be adding a new test in this patch.

The problem is that you're trying to do it all at once. There's no need to implement the full disassembly at once. You can do it in a series of patches, one after the other that build towards the primary goal. For example, there are many if (<X is some value>) type cases, which you could omit - just assume those are all false, in earlier versions, and add them in (with corresponding testing) in a later patch. Similarly, you could assume that all results of x & y return some specific value, e.g. 0, and just print that for now. Yes, that means you won't support everything from the point at which this patch lands, but it will make each individual patch easier to reason with. This fits much better with LLVM's preferred approach - please see https://llvm.org/docs/DeveloperPolicy.html#incremental-development.

Jul 1 2020, 11:20 AM · Restricted Project

Jun 26 2020

scott.linder added inline comments to D57023: [MsgPack] New MsgPackDocument class.
Jun 26 2020, 8:14 PM · Restricted Project
scott.linder accepted D82519: [AMDGPU] Define DWARF encoding for condition code registers.
Jun 26 2020, 9:50 AM · Restricted Project

Jun 24 2020

scott.linder committed rG4d81aec40c62: [MIR] Fix CFI_INSTRUCTION escape printing (authored by scott.linder).
[MIR] Fix CFI_INSTRUCTION escape printing
Jun 24 2020, 3:48 PM
scott.linder closed D82478: [MIR] Fix CFI_INSTRUCTION escape printing.
Jun 24 2020, 3:47 PM · Restricted Project
scott.linder added inline comments to D57023: [MsgPack] New MsgPackDocument class.
Jun 24 2020, 1:01 PM · Restricted Project
scott.linder updated the diff for D82478: [MIR] Fix CFI_INSTRUCTION escape printing.

Simplify test

Jun 24 2020, 10:15 AM · Restricted Project
scott.linder added reviewers for D82478: [MIR] Fix CFI_INSTRUCTION escape printing: thegameg, arsenm.
Jun 24 2020, 10:15 AM · Restricted Project
scott.linder created D82478: [MIR] Fix CFI_INSTRUCTION escape printing.
Jun 24 2020, 9:45 AM · Restricted Project

Jun 22 2020

scott.linder added inline comments to D81780: AMDGPU/AMDHSA: Implement new target ID support in AMDGPU backend.
Jun 22 2020, 11:49 AM · Restricted Project

Jun 18 2020

scott.linder accepted D82090: [MC] Pass the symbol rather than its name to onSymbolStart().
Jun 18 2020, 8:40 AM · Restricted Project

Jun 17 2020

scott.linder committed rG691ff4682f8c: [AMDGPU] Skip CFIInstructions in SIInsertWaitcnts (authored by scott.linder).
[AMDGPU] Skip CFIInstructions in SIInsertWaitcnts
Jun 17 2020, 9:41 AM
scott.linder closed D76881: [AMDGPU] Skip CFIInstructions in SIInsertWaitcnts.
Jun 17 2020, 9:41 AM · debug-info, Restricted Project
scott.linder committed rG2e28009981f9: [NFC] Move getAll{S,V}GPR{32,128} methods to SIFrameLowering (authored by RamNalamothu).
[NFC] Move getAll{S,V}GPR{32,128} methods to SIFrameLowering
Jun 17 2020, 9:41 AM
scott.linder closed D79878: [NFC] Move getAll{S,V}GPR{32,128} methods to SIFrameLowering.
Jun 17 2020, 9:41 AM · Restricted Project

Jun 12 2020

scott.linder accepted D81653: [ELF] Fixing an issue in Elf_Note_Impl::getDescAsStringRef.
Jun 12 2020, 1:42 PM · Restricted Project
scott.linder committed rG480a16d5c809: [MC] Changes to help improve target specific symbol disassembly (authored by rochauha).
[MC] Changes to help improve target specific symbol disassembly
Jun 12 2020, 1:10 PM
scott.linder closed D80512: [MC] Changes to help improve target specific symbol disassembly.
Jun 12 2020, 1:09 PM · Restricted Project

Jun 11 2020

scott.linder accepted D81653: [ELF] Fixing an issue in Elf_Note_Impl::getDescAsStringRef.
Jun 11 2020, 2:22 PM · Restricted Project
scott.linder accepted D80512: [MC] Changes to help improve target specific symbol disassembly.

LGTM, sorry for the delay in reviewing. My feeling is that we should just keep the simple rules you have outlined in the docs and require the callback implementation ensure things are not decoded twice by buffering their output (if needed) and setting Size correctly.

Jun 11 2020, 1:47 PM · Restricted Project

Jun 5 2020

scott.linder added inline comments to D80750: llvm-link: Add module flag behavior MergeTargetID.
Jun 5 2020, 3:10 PM · Restricted Project
scott.linder accepted D81185: [ELF] Adding accessor method for getting Note Desc as StringRef.

I don't know why I didn't add any tests of getDesc() (or anything else) originally, maybe because there is no existing testing and it requires both a unit test and a binary file input? I think at this point we shouldn't block adding an obvious 1-line function on adding tests that should already exist. So, LGTM with Matt's request addressed, and we can add testing in a general way at some point.

Jun 5 2020, 11:12 AM · Restricted Project

Jun 4 2020

scott.linder accepted D81142: [MsgPack] Added a convenience operator.

LGTM, thanks!

Jun 4 2020, 8:44 AM · Restricted Project

Jun 2 2020

scott.linder added a comment to D80249: CodeGen: Don't lazily construct MachineFunctionInfo.

I'm not comfortable accepting this as I'm not familiar with the code, but it LGTM. The pattern of constructing the MachineFunctionInfo lazily on the first call to getInfo seems very non-obvious; it isn't implied by the name and it isn't described in the doc comment. As you note it is also particularly dangerous if it depends on values which are mutable over the life of the MachineFunction.

Jun 2 2020, 10:25 AM · Restricted Project

May 28 2020

scott.linder added inline comments to D80512: [MC] Changes to help improve target specific symbol disassembly.
May 28 2020, 2:18 PM · Restricted Project
scott.linder accepted D79400: [CMAKE] Fix build failure when source directory is read only.

LGTM, thanks!

May 28 2020, 8:43 AM · Restricted Project, Restricted Project

May 27 2020

scott.linder added inline comments to D79400: [CMAKE] Fix build failure when source directory is read only.
May 27 2020, 9:43 AM · Restricted Project, Restricted Project

May 21 2020

scott.linder accepted D79671: [MsgPack] MsgPackDocument::readFromBlob now merges.

LGTM, thank you for bearing with us taking a while to review. I left a small request but feel free to commit without updating the review.

May 21 2020, 8:03 AM · Restricted Project
scott.linder added a comment to D79671: [MsgPack] MsgPackDocument::readFromBlob now merges.
In D79671#2041363, @tpr wrote:

With the nil/empty change, there is a change in behavior in that doing a map lookup "map[key]" with a key not previously in the map creates a new map entry that is empty rather than nil. That is more correct, but I have just discovered some code in LLPC relying on the old behavior. I can fix the LLPC code, but is the change in behavior going to cause any HSA problems?

May 21 2020, 7:31 AM · Restricted Project

May 20 2020

scott.linder added a comment to D78811: [AMDGPU] Enable base pointer..

I posted a piglit test to try to exercise this https://gitlab.freedesktop.org/mesa/piglit/-/merge_requests/291

May 20 2020, 10:54 AM · Restricted Project

May 19 2020

scott.linder accepted D80121: [MsgPack] Added convenience assignment to MsgPackDocument.
May 19 2020, 9:16 AM · Restricted Project

May 15 2020

scott.linder committed rG03c44c7584b4: [NFC] Deduplicate comment in PromoteMemoryToRegister.cpp (authored by scott.linder).
[NFC] Deduplicate comment in PromoteMemoryToRegister.cpp
May 15 2020, 12:32 PM
scott.linder added a comment to D79400: [CMAKE] Fix build failure when source directory is read only.

I'd be interested in the answer concerning why we need to avoid git rev-parse HEAD; it seems like the cleanest solution is to just always check if git rev-parse HEAD changes to determine whether to regenerate the header.

May 15 2020, 10:51 AM · Restricted Project, Restricted Project

May 14 2020

scott.linder updated the diff for D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

Correct test file names. I missed this when renaming the CFA operation itself
to match the updated proposal.

May 14 2020, 1:03 PM · debug-info, Restricted Project
scott.linder updated the diff for D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

Use llvm-readelf for more compact tests

May 14 2020, 1:03 PM · debug-info, Restricted Project
scott.linder added inline comments to D76879: [AMDGPU] Begin emitting CFI for AMDGCN.
May 14 2020, 1:03 PM · debug-info, Restricted Project
scott.linder updated the diff for D76879: [AMDGPU] Begin emitting CFI for AMDGCN.
  • Use -COUNT-N form
  • Use '#' comment character
  • Use llvm-readelf for more compact tests
  • Refactor lit config test
May 14 2020, 1:03 PM · debug-info, Restricted Project
scott.linder updated the diff for D76884: [AMDGPU] Implement -amdgpu-spill-cfi-saved-regs.

Refer to subreg index directly

May 14 2020, 11:57 AM · debug-info, Restricted Project
scott.linder accepted D78811: [AMDGPU] Enable base pointer..

This will require some changes to the CFI, although previously this case was broken anyway so it does not regress the unwind information. I think we will just need to define the CFA relative to the base pointer (if it is actually set up) or the stack pointer (if realignment is needed, but the base pointer is not).

May 14 2020, 11:57 AM · Restricted Project
scott.linder updated the diff for D76883: [AMDGPU] Implement CFI for CSR spills.

Re-add a test that got lost in a rebase somewhere

May 14 2020, 10:50 AM · debug-info, Restricted Project

May 13 2020

scott.linder added inline comments to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 13 2020, 4:57 PM · debug-info, Restricted Project
scott.linder added inline comments to D78778: Add SupportsDebugUnwindInformation to MCAsmInfo.
May 13 2020, 3:51 PM · debug-info, Restricted Project
scott.linder added inline comments to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 13 2020, 2:10 PM · debug-info, Restricted Project
scott.linder updated the diff for D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

Add tests for parse errors, fix formatting

May 13 2020, 2:10 PM · debug-info, Restricted Project
scott.linder accepted D79776: [AMDGPU] Allow use of StackPtrOffsetReg when building spills.

LGTM, although CHECKs in the tests to confirm that e.g. the first involves a scavenged register and s_add, and the second involves the stack pointer and s_mov, and that this happens at the expected offset might make it easier to read and more likely to catch something that breaks this in the future. Right now it isn't clear why there are two nearly identical tests.

May 13 2020, 1:03 PM · Restricted Project
scott.linder added a comment to D79671: [MsgPack] MsgPackDocument::readFromBlob now merges.
In D79671#2032032, @tpr wrote:

Dinesh is working on merging metadata in ROCm CompilerSupport using MsgPackDocument, and it fell out that merging YAML metadata was also handled transparently. What is the advantage to doing this via a function pointer passed to readFromBlob? It doesn't seem to save any copies as you still need to be able to compare the nodes; is it just a matter of readability? Would it be reasonable to also add a callback for fromYAML?

I too tried doing it by reading the second blob into a MsgPackDocument and merging it into the first one from there. But I think that does involve more copies/allocation/etc because a map in the second document would actually get created as a std::map before having its elements merged into the corresponding map in the first document.

I guess the same thing as I have done could be done for the YAML reading code.

May 13 2020, 11:25 AM · Restricted Project
scott.linder updated the diff for D76882: [AMDGPU] Implement CFI for non-kernel functions.

Rebase over https://reviews.llvm.org/D79878

May 13 2020, 9:44 AM · debug-info, Restricted Project
scott.linder created D79878: [NFC] Move getAll{S,V}GPR{32,128} methods to SIFrameLowering.
May 13 2020, 9:44 AM · Restricted Project
scott.linder updated the diff for D76884: [AMDGPU] Implement -amdgpu-spill-cfi-saved-regs.

Rebase

May 13 2020, 8:38 AM · debug-info, Restricted Project
scott.linder updated the diff for D76883: [AMDGPU] Implement CFI for CSR spills.

Rebase

May 13 2020, 8:37 AM · debug-info, Restricted Project

May 12 2020

scott.linder added a reviewer for D76882: [AMDGPU] Implement CFI for non-kernel functions: kerbowa.
May 12 2020, 5:15 PM · debug-info, Restricted Project
scott.linder updated the diff for D76882: [AMDGPU] Implement CFI for non-kernel functions.

Rebase, generalize the previously hard-coded list of caller-saved registers
with help from @cdevadas, and emit CFI for the new case where we spill the FP
to VMEM.

May 12 2020, 5:15 PM · debug-info, Restricted Project
scott.linder updated the diff for D76880: [AMDGPU] Emit entry function CFI.

Rebase

May 12 2020, 1:27 PM · debug-info, Restricted Project
scott.linder updated the diff for D76879: [AMDGPU] Begin emitting CFI for AMDGCN.

Rebase and update summary

May 12 2020, 12:54 PM · debug-info, Restricted Project
scott.linder updated the diff for D76878: Implement DW_{OP,AT}_LLVM_* for Heterogeneous Debugging.

Rebase and move tests to .debug_info as some new operations are not legal in
.debug_frame.

May 12 2020, 11:49 AM · debug-info, Restricted Project
scott.linder added a project to D78778: Add SupportsDebugUnwindInformation to MCAsmInfo: debug-info.
May 12 2020, 11:49 AM · debug-info, Restricted Project
scott.linder added inline comments to D78778: Add SupportsDebugUnwindInformation to MCAsmInfo.
May 12 2020, 11:49 AM · debug-info, Restricted Project
scott.linder updated the diff for D78778: Add SupportsDebugUnwindInformation to MCAsmInfo.

Rebase

May 12 2020, 11:48 AM · debug-info, Restricted Project
scott.linder updated the diff for D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

Address feedback

May 12 2020, 11:16 AM · debug-info, Restricted Project

May 11 2020

scott.linder added a reviewer for D79671: [MsgPack] MsgPackDocument::readFromBlob now merges: dineshkb-amd.

Dinesh is working on merging metadata in ROCm CompilerSupport using MsgPackDocument, and it fell out that merging YAML metadata was also handled transparently. What is the advantage to doing this via a function pointer passed to readFromBlob? It doesn't seem to save any copies as you still need to be able to compare the nodes; is it just a matter of readability? Would it be reasonable to also add a callback for fromYAML?

May 11 2020, 7:59 PM · Restricted Project
scott.linder added a comment to D78778: Add SupportsDebugUnwindInformation to MCAsmInfo.

@scott.linder is this not yet ready for review, as you have not added any reviewers?

May 11 2020, 6:55 PM · debug-info, Restricted Project
scott.linder added reviewers for D78778: Add SupportsDebugUnwindInformation to MCAsmInfo: espindola, jhenderson, t-tye, RamNalamothu, dblaikie, aprantl, djtodoro, ostannard, probinson, ikudrin.
May 11 2020, 6:55 PM · debug-info, Restricted Project

May 8 2020

scott.linder added inline comments to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 8 2020, 3:03 PM · debug-info, Restricted Project

May 6 2020

scott.linder added a comment to D79400: [CMAKE] Fix build failure when source directory is read only.

Thank you for the explanation; I'm also still lost as to why .git/logs/HEAD is referenced at all then. I would propose removing find_first_existing_vc_file entirely, as it seems to be completely redundant if there is another mechanism for ensuring the VCS header is up-to-date without triggering gratuitous rebuilds, but I will defer to any of the other reviewers on the blame for the scripts.

May 6 2020, 9:40 AM · Restricted Project, Restricted Project

May 5 2020

scott.linder added a comment to D79400: [CMAKE] Fix build failure when source directory is read only.

Can you provide more context in the diff? Looking at the current master locally it seems like this patch changes the behavior of the llvm_vcsrevision_h target when logs/HEAD isn't found, such that it only depends on the generation script. I.e. if you configure CMake without the HEAD file present, and then you do something which changes the working directory and creates HEAD the target will not be out of date and won't be rebuilt. Before this patch it would have noticed the change to HEAD and forced regenerating the version header.

May 5 2020, 2:03 PM · Restricted Project, Restricted Project

Apr 30 2020

scott.linder committed rG084f3cf92b95: [AMDGPU] Update DWARF proposal encodings (authored by scott.linder).
[AMDGPU] Update DWARF proposal encodings
Apr 30 2020, 11:16 AM