[PATCH] D126177: [BOLT] [AArch64] Handle data markers spanning multiple functions

Thu May 26 00:59:50 PDT 2022

treapster added inline comments.

================
Comment at: bolt/test/AArch64/unmarked-data.s:5-9
+// COM: We make sure that symbol second is not marked as data by compiler.
+// COM: We find it's address, then find all symbols at that address and check there is no $d marker
+// RUN: out=`llvm-readelf -Ws %t.exe | grep second`; str=($(echo "$out" | tr '\t' '\n')); addr=${str[1]}; \
+// RUN: llvm-readelf -Ws %t.exe | grep $addr | FileCheck %s
+// CHECK-NOT: $d
----------------
rafauler wrote:
> While I do appreciate unix style command line processing, I feel we should be careful with this in case we run this in an environment without shell (e.g. Windows). I'm not an expert in Windows support and we currently do not have a Windows buildbot for BOLT, but I feel this would probably fail in Visual Studio.
> 
> We should probably remove these lines too as, from a test perspective, this is enforcing that llvm-mc and ld.lld always behave in that specific way regarding emission of "$d" symbols. If somebody tries to change that, this test will break, even though they are not touching BOLT. Here we should limit ourselves to test BOLT.
I totally agree with you that we should avoid depending on shell or specific behavior of assembler/linker, but the only way i see to remove this dependency is to use fixed prebuilt binary with omitted markers. As far as i know, such tests are discouraged in llvm, but we can probably use yaml with yaml2obj? I'll try to recreate it in yaml.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126177