[BOLT][DWARF] Add version 5 split dwarf support

[ayermolo]

Summary

Added support for DWARF5 Split Dwarf.

[ayermolo]

rebase

[ayermolo]

test

[ayermolo]

rebase

[ayermolo]

nit

[dblaikie]

Probably worth changing the == 5 version checks to >= 5 since most things stay the same between DWARF versions, and the things that don't are the exceptions/things we'll need to touch on the next version (& nice to not have to touch/update code for things that stay the same)

bolt/include/bolt/Core/DebugData.h

375–377 ↗

(On Diff #423161)

That seems like a somewhat problematic API - returning two very different kinds of data (the DWO_ID or the unit offset) seems quite easy to misuse this?

386 ↗

(On Diff #423161)

Probably name the parameter the same as the member, rather than one character different/lower case.

bolt/lib/Rewrite/DWARFRewriter.cpp

208–209 ↗

(On Diff #423161)

Probably an uppercase V since that's a separate word within the name?

306 ↗

(On Diff #423161)

320 ↗

(On Diff #423161)

reinterpret_cast seems a bit scary here - perhaps TempRangesSectionWriter should be a void* if it could be a pointer to two unrelated types (if it's a pointer to a base class that covers both ranges and rnglists, then a static_cast should suffice for the downcasting from base pointer to derived pointer)

327–328 ↗

(On Diff #423161)

to avoid two lookups (one for count, the other for op[]) perhaps this code should use find?

[ayermolo]

Comments from @maksfb

bolt/include/bolt/Core/DebugData.h

219 ↗

(On Diff #423161)

delete

230 ↗

(On Diff #423161)

make reference

348 ↗

(On Diff #423161)

add documentation.

386 ↗

(On Diff #423161)

Pass BC by reference

399 ↗

(On Diff #423161)

isFinalized

402 ↗

(On Diff #423161)

releaseBuffer()

bolt/lib/Core/DebugData.cpp

210 ↗

(On Diff #423161)

const

358 ↗

(On Diff #423161)

const

417 ↗

(On Diff #423161)

AM --> AMIter

483 ↗

(On Diff #423161)

const
getCUId -->getCUID

1058 ↗

(On Diff #423161)

remove method, move to constructor

1069 ↗

(On Diff #423161)

const

1071 ↗

(On Diff #423161)

Darf --> Dwarf

1075 ↗

(On Diff #423161)

use static_cast

1093 ↗

(On Diff #423161)

remove curly braces

bolt/lib/Rewrite/DWARFRewriter.cpp

166 ↗

(On Diff #423161)

const Unit, AttrInfoVal

168 ↗

(On Diff #423161)

const

208 ↗

(On Diff #423161)

const DwarfVersion

304 ↗

(On Diff #423161)

add else if for DWARF < 5 else case llvm_unreachable

322 ↗

(On Diff #423161)

make finalizeSection virtual and do nothing for dwarf4.

335 ↗

(On Diff #423161)

Make initSection virtual and do nothing for DWARF4

1175 ↗

(On Diff #423161)

1719 ↗

(On Diff #423161)

const

bolt/test/X86/dwarf4-df-dualcu.test

23 ↗

(On Diff #423161)

dwarf4

[ayermolo]

addressing comments

[ayermolo]

oops forgot to switch branches

[ayermolo]

bolt/include/bolt/Core/DebugData.h

375–377 ↗

(On Diff #423161)

The idea I had behind this APIS is for it to return unique ID representing the CU. As it applies to .debug_addr. For monolithic case is its offset with .debug_info for Split Dwarf case is its DWO ID. At least in my head viewed in that context the return data is the same. It's just something that uniquely identifies this CU, and logic is encapsulated in it.

[ayermolo]

bolt/include/bolt/Core/DebugData.h

375–377 ↗

(On Diff #423161)

@dblaikie What do you think?

[dblaikie]

bolt/include/bolt/Core/DebugData.h

375–377 ↗

(On Diff #423161)

Could you use the offset consistently? The debug_addr section is always in the executable - the offset of the skeleton CU would be unique?

[ayermolo]

bolt/include/bolt/Core/DebugData.h

375–377 ↗

(On Diff #423161)

In theory yes, if I always use offset of the CU/Skeleton CU in the main binary. For that I still need to do a check of CU that is passed in is DWO CU, get it skeleton CU, then get it's offset. At the end what we get is offset all the time. Is it really better? Sure we always getting an offset, but intent of this API is not really get an offset but the unique ID of the CU. Are you concern that offset and DWO ID will collide?

[ayermolo]

updated so that getCUID consistently returns CU/SkeletonCU offset.

[maksfb]

Awesome!

cross-project-tests/lit.cfg.py

 
 # clang_src_dir and lld_src_dir are not used by these tests, but are required by
 # use_clang() and use_lld() respectively, so set them to "", if needed.
 if not hasattr(config, 'clang_src_dir'):
     config.clang_src_dir = ""
 # Facebook T92898286
 should_test_bolt = get_required_attr(config, "llvm_test_bolt")
 if should_test_bolt:
-    llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), additional_flags=['--post-link-optimize', '-fdebug-default-version=4'])
+    llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), additional_flags=['--post-link-optimize'])
 else:
     llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects))
 # End Facebook T92898286
 
 if not hasattr(config, 'lld_src_dir'):
     config.lld_src_dir = ""
 llvm_config.use_lld(required=('lld' in config.llvm_enabled_projects))
 

lldb/test/API/lit.cfg.py

 if is_configured('lldb_libs_dir'):
   dotest_cmd += ['--lldb-libs-dir', config.lldb_libs_dir]
 
 if is_configured('lldb_framework_dir'):
   dotest_cmd += ['--framework', config.lldb_framework_dir]
 
 # Facebook T92898286
 if is_configured("llvm_test_bolt"):
-    dotest_cmd += ['-E', '"--post-link-optimize"',
-                   '-E', '"-fdebug-default-version=4"']
+    dotest_cmd += ['-E', '"--post-link-optimize"']
 # End Facebook T92898286
 
 if 'lldb-repro-capture' in config.available_features or \
     'lldb-repro-replay' in config.available_features:
   dotest_cmd += ['--skip-category=lldb-vscode', '--skip-category=std-module']
 
 if 'lldb-simulator-ios' in config.available_features:
   dotest_cmd += ['--apple-sdk', 'iphonesimulator',

lldb/test/Shell/helper/toolchain.py

if sys.platform.startswith('netbsd'):
         host_flags += ['-L' + config.llvm_libs_dir,
                   '-Wl,-rpath,' + config.llvm_libs_dir]
 
     # The clang module cache is used for building inferiors.
     host_flags += ['-fmodules-cache-path={}'.format(config.clang_module_cache)]
 
     # Facebook T92898286
     if config.llvm_test_bolt:
-        host_flags += ['--post-link-optimize', '-fdebug-default-version=4']
+        host_flags += ['--post-link-optimize']
     # End Facebook T92898286
 
     host_flags = ' '.join(host_flags)
     config.substitutions.append(('%clang_host', '%clang ' + host_flags))
     config.substitutions.append(('%clangxx_host', '%clangxx ' + host_flags))
     config.substitutions.append(('%clang_cl_host', '%clang_cl --target='+config.host_triple))
 
     additional_tool_dirs=[]