[PATCH] D143841: [Propeller] Make decoding BBAddrMaps trace through relocations

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added inline comments.


================
Comment at: llvm/lib/Object/ELFObjectFile.cpp:710
+  for (auto const &[Sec, RelocSec] : *SectionRelocMapOrErr) {
+    std::optional<Elf_Shdr> RelaSec = {};
+    if (IsRelocatable)
----------------
`std::nullopt`.


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml:43-54
+          - ID:              1
+            AddressOffset:   0x0
+            Size:            0xE
+            Metadata:        0x0
+          - ID:              2
+            AddressOffset:   0x0
+            Size:            0x6
----------------
You can also remove these extra blocks to shorten the test.


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml:79-88
+# ATT:      <a>:
+# ATT-NEXT: <BB0>:
+# ATT-NEXT:   pushq   %rbp
+# ATT-NEXT:   movq    %rsp, %rbp
+# ATT-NEXT:   movl    %edi, -0x4(%rbp)
+# ATT-NEXT:   movl    -0x4(%rbp), %eax
+# ATT-NEXT:   shll    $0x1, %eax
----------------
This assembly is irrelevant. right? I mean the object file can be synthesized even if it's not from real code. In that case, I suggest we remove the assembly code.


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml:5
+
+# UNSUPPORTED: system-windows
+
----------------
aidengrossman wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > Why is this unsupported on Windows?
> > Given we have an open ticket for this, I wonder if this should be an XFAIL, not UNSUPPORTED. The idea being that when the issue is fixed, the test will start x-passing, and not be forgotten about.
> I think we should stick with `UNSUPPORTED` here because the original failure was due to some change specific to VS2022, and the minimum version to build LLVM is currently VS2019. This means that there might be some buildbots/developers/users building LLVM on VS2019 where this test would unexpectedly pass.
In fact, current build bots still use VS2019 which makes the test succeed on Windows too. The issue with VS2022 was revealed later upon downstream builds with VS2022.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143841/new/

https://reviews.llvm.org/D143841