Commit 96fe3c3
committed
Auto merge of rust-lang#147022 - Zalathar:no-args, r=wesleywiser
Remove current code for embedding command-line args in PDB
The compiler currently has code that will obtain a list of quoted command-line arguments, and pass it through to TargetMachine creation, so that the command-line args can be embedded in PDB output.
This PR removes that code, due to subtle concerns that might not have been apparent when it was originally added.
---
Those concerns include:
- The entire command-line quoting process is repeated every time a target-machine-factory is created. In incremental builds this typically occurs 500+ times, instead of happening only once. The repeated quoting constitutes a large chunk of instructions executed in the `large-workspace` benchmark.
- See rust-lang#146804 (comment) for an example of the perf consequences of skipping all that work.
- This overhead occurs even when building for targets or configurations that don't emit PDB output.
- Command-line arguments are obtained in a way that completely bypasses the query system, which is a problem for the integrity of incremental compilation.
- Fixing this alone is likely to inhibit incremental rebuilds for most or all CGUs, even in builds that don't emit PDB output.
- Command-line arguments and the executable path are obtained in a way that completely bypasses the compiler's path-remapping system, which is a reproducibility hazard.
- rust-lang#128842
---
Relevant PRs:
- rust-lang#113492
- rust-lang#130446
- rust-lang#131805
- rust-lang#146700
- rust-lang#146973
Zulip thread:
- https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Some.20PDB.20info.20bypasses.20the.20query.20system.20and.20path.20remapping/with/541432211
---
According to rust-lang#96475, one of the big motivations for embedding the command-line arguments was to enable tools like Live++. [It appears that Live++ doesn't actually support Rust yet](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/embeded.20compiler.20args.20and.20--remap-path-prefix/near/523800010), so it's possible that there aren't any existing workflows for this removal to break.
In the future, there could be a case for reintroducing some or all of this functionality, guarded behind an opt-in flag so that it doesn't cause problems for other users. But as it stands, the current implementation puts a disproportionate burden on other users and on compiler maintainers.File tree
19 files changed
+1
-162
lines changed- compiler
- rustc_codegen_llvm/src
- back
- command_line_args
- llvm
- rustc_codegen_ssa/src/back
- rustc_driver_impl/src
- rustc_interface/src
- rustc_llvm/llvm-wrapper
- rustc_session/src
- src/librustdoc
- tests
- run-make/pdb-buildinfo-cl-cmd
- ui-fulldeps
19 files changed
+1
-162
lines changedLines changed: 0 additions & 37 deletions
This file was deleted.
Lines changed: 0 additions & 25 deletions
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
3 | 2 | | |
4 | 3 | | |
5 | 4 | | |
| |||
Lines changed: 0 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
42 | | - | |
43 | 41 | | |
44 | 42 | | |
45 | 43 | | |
| |||
66 | 64 | | |
67 | 65 | | |
68 | 66 | | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | 67 | | |
74 | 68 | | |
75 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
35 | 34 | | |
36 | 35 | | |
37 | 36 | | |
| |||
253 | 252 | | |
254 | 253 | | |
255 | 254 | | |
256 | | - | |
257 | | - | |
258 | | - | |
259 | | - | |
260 | | - | |
261 | | - | |
262 | | - | |
263 | | - | |
264 | | - | |
265 | | - | |
266 | | - | |
267 | | - | |
268 | | - | |
269 | 255 | | |
270 | 256 | | |
271 | 257 | | |
| |||
326 | 312 | | |
327 | 313 | | |
328 | 314 | | |
329 | | - | |
330 | | - | |
331 | 315 | | |
332 | 316 | | |
333 | 317 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2330 | 2330 | | |
2331 | 2331 | | |
2332 | 2332 | | |
2333 | | - | |
2334 | | - | |
2335 | | - | |
2336 | | - | |
2337 | 2333 | | |
2338 | 2334 | | |
2339 | 2335 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
346 | 346 | | |
347 | 347 | | |
348 | 348 | | |
349 | | - | |
350 | | - | |
351 | | - | |
352 | | - | |
353 | | - | |
354 | | - | |
355 | 349 | | |
356 | 350 | | |
357 | 351 | | |
| |||
1153 | 1147 | | |
1154 | 1148 | | |
1155 | 1149 | | |
1156 | | - | |
1157 | 1150 | | |
1158 | 1151 | | |
1159 | 1152 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
269 | 269 | | |
270 | 270 | | |
271 | 271 | | |
272 | | - | |
273 | 272 | | |
274 | 273 | | |
275 | 274 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
376 | 376 | | |
377 | 377 | | |
378 | 378 | | |
379 | | - | |
380 | | - | |
381 | | - | |
382 | | - | |
383 | | - | |
384 | | - | |
385 | 379 | | |
386 | 380 | | |
387 | 381 | | |
| |||
480 | 474 | | |
481 | 475 | | |
482 | 476 | | |
483 | | - | |
484 | 477 | | |
485 | 478 | | |
486 | 479 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
81 | | - | |
82 | 81 | | |
83 | 82 | | |
84 | 83 | | |
| |||
0 commit comments