[PATCH] D59020: [StackMaps] Update llvm-readobj to parse V3 Stackmaps

Mon Mar 11 10:55:56 PDT 2019

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM if you're confident in the correctness of the Register -> Indirect changes commented on, otherwise, let's discuss.

================
Comment at: llvm/test/Object/stackmap-dump.test:1
-RUN: llvm-readobj -stackmap %p/Inputs/stackmap-test.macho-x86-64 | FileCheck %s
+RUN: llvm-readobj -stackmap %p/Inputs/stackmap-test.elf-x86-64 | FileCheck %s

----------------
jacob-hughes wrote:
> reames wrote:
> > Rather than switching from macho to elf, please add an extra run line for ELF.  (i.e. test both)
> The reason for this change was that the macho file was created back when v2 stackmaps would have been generated. I recompiled the .ll with with the most recent version of LLVM on my machine (linux) and am not entirely sure how to cross-compile to macho.
> 
> If you think it's necessary to have an up to date macho object file too, then I can look into how to make one.
Ah, that makes sense.

Given we're effectively removing the V2 parser, and replacing it with a V3 parser, I'm fine with the switch to ELF.  Just do me a favour and add a comment to the top of the file about how when the parser format changes, the object file input needs regenerated.  

Ideally, I'd like to find a way to reduce the diff (i.e. remove the Macho->ELF pieces and leave only the version pieces), but I don't see an obvious way to do so.  

================
Comment at: llvm/test/Object/stackmap-dump.test:106
+CHECK-NEXT:         #1: Register R#0
+CHECK-NEXT:         #2: Indirect [R#6 + 16]
+CHECK-NEXT:         #3: Indirect [R#6 + 24]
----------------
These Register to Indirect changes bug me.  Do you know why these changed?

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

https://reviews.llvm.org/D59020