Skip to content

Commit 72a28e8

Browse files
committed
Improve ModuleConfig initialization.
There are three `ModuleConfigs`, one for each `ModuleKind`. The code to initialized them is spaghetti imperative code that sets each field to a default value and then modifies many fields in complicated ways. This makes it very hard to tell what value ends up in each field in each config. For example, the `modules_config.emit_pre_lto_bc` field is set twice, which means it can be set to true and then incorrectly set back to false. (This probably hasn't been noticed because it happens in a very obscure case.) This commit changes the code to a declarative style in which `ModuleConfig::new` initializes all fields and then they are never changed again. This is slightly more concise and much easier to read. (And it fixes the abovementioned `emit_pre_lto_bc` error as well.)
1 parent 4c8bf50 commit 72a28e8

File tree

1 file changed

+137
-152
lines changed

1 file changed

+137
-152
lines changed

src/librustc_codegen_ssa/back/write.rs

Lines changed: 137 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ pub struct ModuleConfig {
104104
pub emit_ir: bool,
105105
pub emit_asm: bool,
106106
pub emit_obj: EmitObj,
107+
107108
// Miscellaneous flags. These are mostly copied from command-line
108109
// options.
109110
pub verify_llvm_ir: bool,
@@ -118,77 +119,144 @@ pub struct ModuleConfig {
118119
}
119120

120121
impl ModuleConfig {
121-
fn new(passes: Vec<String>) -> ModuleConfig {
122-
ModuleConfig {
123-
passes,
124-
opt_level: None,
125-
opt_size: None,
126-
127-
pgo_gen: SwitchWithOptPath::Disabled,
128-
pgo_use: None,
129-
130-
sanitizer: None,
131-
sanitizer_recover: Default::default(),
132-
sanitizer_memory_track_origins: 0,
133-
134-
emit_no_opt_bc: false,
135-
emit_pre_lto_bc: false,
136-
emit_bc: false,
137-
emit_bc_compressed: false,
138-
emit_ir: false,
139-
emit_asm: false,
140-
emit_obj: EmitObj::None,
141-
142-
verify_llvm_ir: false,
143-
no_prepopulate_passes: false,
144-
no_builtins: false,
145-
time_module: true,
146-
vectorize_loop: false,
147-
vectorize_slp: false,
148-
merge_functions: false,
149-
inline_threshold: None,
150-
new_llvm_pass_manager: None,
122+
fn new(kind: ModuleKind, sess: &Session, no_builtins: bool) -> ModuleConfig {
123+
// If it's a regular module, use `$regular`, otherwise use `$other`.
124+
// `$regular` and `$other` are evaluated lazily.
125+
macro_rules! if_regular {
126+
($regular: expr, $other: expr) => {
127+
if let ModuleKind::Regular = kind { $regular } else { $other }
128+
};
151129
}
152-
}
153130

154-
fn set_flags(&mut self, sess: &Session, no_builtins: bool) {
155-
self.verify_llvm_ir = sess.verify_llvm_ir();
156-
self.no_prepopulate_passes = sess.opts.cg.no_prepopulate_passes;
157-
self.no_builtins = no_builtins || sess.target.target.options.no_builtins;
158-
self.inline_threshold = sess.opts.cg.inline_threshold;
159-
self.new_llvm_pass_manager = sess.opts.debugging_opts.new_llvm_pass_manager;
160-
161-
// Copy what clang does by turning on loop vectorization at O2 and
162-
// slp vectorization at O3. Otherwise configure other optimization aspects
163-
// of this pass manager builder.
164-
self.vectorize_loop = !sess.opts.cg.no_vectorize_loops
165-
&& (sess.opts.optimize == config::OptLevel::Default
166-
|| sess.opts.optimize == config::OptLevel::Aggressive);
167-
168-
self.vectorize_slp =
169-
!sess.opts.cg.no_vectorize_slp && sess.opts.optimize == config::OptLevel::Aggressive;
170-
171-
// Some targets (namely, NVPTX) interact badly with the MergeFunctions
172-
// pass. This is because MergeFunctions can generate new function calls
173-
// which may interfere with the target calling convention; e.g. for the
174-
// NVPTX target, PTX kernels should not call other PTX kernels.
175-
// MergeFunctions can also be configured to generate aliases instead,
176-
// but aliases are not supported by some backends (again, NVPTX).
177-
// Therefore, allow targets to opt out of the MergeFunctions pass,
178-
// but otherwise keep the pass enabled (at O2 and O3) since it can be
179-
// useful for reducing code size.
180-
self.merge_functions = match sess
181-
.opts
182-
.debugging_opts
183-
.merge_functions
184-
.unwrap_or(sess.target.target.options.merge_functions)
131+
let opt_level_and_size = if_regular!(Some(sess.opts.optimize), None);
132+
133+
let save_temps = sess.opts.cg.save_temps;
134+
135+
let should_emit_obj = sess.opts.output_types.contains_key(&OutputType::Exe)
136+
|| match kind {
137+
ModuleKind::Regular => sess.opts.output_types.contains_key(&OutputType::Object),
138+
ModuleKind::Allocator => false,
139+
ModuleKind::Metadata => sess.opts.output_types.contains_key(&OutputType::Metadata),
140+
};
141+
142+
let emit_obj = if !should_emit_obj {
143+
EmitObj::None
144+
} else if sess.target.target.options.obj_is_bitcode
145+
|| sess.opts.cg.linker_plugin_lto.enabled()
185146
{
186-
MergeFunctions::Disabled => false,
187-
MergeFunctions::Trampolines | MergeFunctions::Aliases => {
188-
sess.opts.optimize == config::OptLevel::Default
189-
|| sess.opts.optimize == config::OptLevel::Aggressive
147+
EmitObj::Bitcode
148+
} else if sess.opts.debugging_opts.embed_bitcode {
149+
match sess.opts.optimize {
150+
config::OptLevel::No | config::OptLevel::Less => {
151+
EmitObj::ObjectCode(BitcodeSection::Marker)
152+
}
153+
_ => EmitObj::ObjectCode(BitcodeSection::Full),
190154
}
155+
} else {
156+
EmitObj::ObjectCode(BitcodeSection::None)
191157
};
158+
159+
ModuleConfig {
160+
passes: if_regular!(
161+
{
162+
let mut passes = sess.opts.cg.passes.clone();
163+
if sess.opts.debugging_opts.profile {
164+
passes.push("insert-gcov-profiling".to_owned());
165+
}
166+
passes
167+
},
168+
vec![]
169+
),
170+
171+
opt_level: opt_level_and_size,
172+
opt_size: opt_level_and_size,
173+
174+
pgo_gen: if_regular!(
175+
sess.opts.cg.profile_generate.clone(),
176+
SwitchWithOptPath::Disabled
177+
),
178+
pgo_use: if_regular!(sess.opts.cg.profile_use.clone(), None),
179+
180+
sanitizer: if_regular!(sess.opts.debugging_opts.sanitizer.clone(), None),
181+
sanitizer_recover: if_regular!(
182+
sess.opts.debugging_opts.sanitizer_recover.clone(),
183+
vec![]
184+
),
185+
sanitizer_memory_track_origins: if_regular!(
186+
sess.opts.debugging_opts.sanitizer_memory_track_origins,
187+
0
188+
),
189+
190+
emit_pre_lto_bc: if_regular!(
191+
save_temps || need_pre_lto_bitcode_for_incr_comp(sess),
192+
false
193+
),
194+
emit_no_opt_bc: if_regular!(save_temps, false),
195+
emit_bc: if_regular!(
196+
save_temps || sess.opts.output_types.contains_key(&OutputType::Bitcode),
197+
save_temps
198+
),
199+
emit_bc_compressed: match kind {
200+
ModuleKind::Regular | ModuleKind::Allocator => {
201+
// Emit compressed bitcode files for the crate if we're
202+
// emitting an rlib. Whenever an rlib is created, the
203+
// bitcode is inserted into the archive in order to allow
204+
// LTO against it.
205+
need_crate_bitcode_for_rlib(sess)
206+
}
207+
ModuleKind::Metadata => false,
208+
},
209+
emit_ir: if_regular!(
210+
sess.opts.output_types.contains_key(&OutputType::LlvmAssembly),
211+
false
212+
),
213+
emit_asm: if_regular!(
214+
sess.opts.output_types.contains_key(&OutputType::Assembly),
215+
false
216+
),
217+
emit_obj,
218+
219+
verify_llvm_ir: sess.verify_llvm_ir(),
220+
no_prepopulate_passes: sess.opts.cg.no_prepopulate_passes,
221+
no_builtins: no_builtins || sess.target.target.options.no_builtins,
222+
223+
// Exclude metadata and allocator modules from time_passes output,
224+
// since they throw off the "LLVM passes" measurement.
225+
time_module: if_regular!(true, false),
226+
227+
// Copy what clang does by turning on loop vectorization at O2 and
228+
// slp vectorization at O3.
229+
vectorize_loop: !sess.opts.cg.no_vectorize_loops
230+
&& (sess.opts.optimize == config::OptLevel::Default
231+
|| sess.opts.optimize == config::OptLevel::Aggressive),
232+
vectorize_slp: !sess.opts.cg.no_vectorize_slp
233+
&& sess.opts.optimize == config::OptLevel::Aggressive,
234+
235+
// Some targets (namely, NVPTX) interact badly with the
236+
// MergeFunctions pass. This is because MergeFunctions can generate
237+
// new function calls which may interfere with the target calling
238+
// convention; e.g. for the NVPTX target, PTX kernels should not
239+
// call other PTX kernels. MergeFunctions can also be configured to
240+
// generate aliases instead, but aliases are not supported by some
241+
// backends (again, NVPTX). Therefore, allow targets to opt out of
242+
// the MergeFunctions pass, but otherwise keep the pass enabled (at
243+
// O2 and O3) since it can be useful for reducing code size.
244+
merge_functions: match sess
245+
.opts
246+
.debugging_opts
247+
.merge_functions
248+
.unwrap_or(sess.target.target.options.merge_functions)
249+
{
250+
MergeFunctions::Disabled => false,
251+
MergeFunctions::Trampolines | MergeFunctions::Aliases => {
252+
sess.opts.optimize == config::OptLevel::Default
253+
|| sess.opts.optimize == config::OptLevel::Aggressive
254+
}
255+
},
256+
257+
inline_threshold: sess.opts.cg.inline_threshold,
258+
new_llvm_pass_manager: sess.opts.debugging_opts.new_llvm_pass_manager,
259+
}
192260
}
193261

194262
pub fn bitcode_needed(&self) -> bool {
@@ -353,92 +421,9 @@ pub fn start_async_codegen<B: ExtraBackendMethods>(
353421
let linker_info = LinkerInfo::new(tcx);
354422
let crate_info = CrateInfo::new(tcx);
355423

356-
// Figure out what we actually need to build.
357-
let mut modules_config = ModuleConfig::new(sess.opts.cg.passes.clone());
358-
let mut metadata_config = ModuleConfig::new(vec![]);
359-
let mut allocator_config = ModuleConfig::new(vec![]);
360-
361-
if sess.opts.debugging_opts.profile {
362-
modules_config.passes.push("insert-gcov-profiling".to_owned())
363-
}
364-
365-
modules_config.pgo_gen = sess.opts.cg.profile_generate.clone();
366-
modules_config.pgo_use = sess.opts.cg.profile_use.clone();
367-
modules_config.sanitizer = sess.opts.debugging_opts.sanitizer.clone();
368-
modules_config.sanitizer_recover = sess.opts.debugging_opts.sanitizer_recover.clone();
369-
modules_config.sanitizer_memory_track_origins =
370-
sess.opts.debugging_opts.sanitizer_memory_track_origins;
371-
modules_config.opt_level = Some(sess.opts.optimize);
372-
modules_config.opt_size = Some(sess.opts.optimize);
373-
374-
// Save all versions of the bytecode if we're saving our temporaries.
375-
if sess.opts.cg.save_temps {
376-
modules_config.emit_no_opt_bc = true;
377-
modules_config.emit_pre_lto_bc = true;
378-
modules_config.emit_bc = true;
379-
metadata_config.emit_bc = true;
380-
allocator_config.emit_bc = true;
381-
}
382-
383-
// Emit compressed bitcode files for the crate if we're emitting an rlib.
384-
// Whenever an rlib is created, the bitcode is inserted into the archive in
385-
// order to allow LTO against it.
386-
if need_crate_bitcode_for_rlib(sess) {
387-
modules_config.emit_bc_compressed = true;
388-
allocator_config.emit_bc_compressed = true;
389-
}
390-
391-
let emit_obj =
392-
if sess.target.target.options.obj_is_bitcode || sess.opts.cg.linker_plugin_lto.enabled() {
393-
EmitObj::Bitcode
394-
} else if sess.opts.debugging_opts.embed_bitcode {
395-
match sess.opts.optimize {
396-
config::OptLevel::No | config::OptLevel::Less => {
397-
EmitObj::ObjectCode(BitcodeSection::Marker)
398-
}
399-
_ => EmitObj::ObjectCode(BitcodeSection::Full),
400-
}
401-
} else {
402-
EmitObj::ObjectCode(BitcodeSection::None)
403-
};
404-
405-
modules_config.emit_pre_lto_bc = need_pre_lto_bitcode_for_incr_comp(sess);
406-
407-
for output_type in sess.opts.output_types.keys() {
408-
match *output_type {
409-
OutputType::Bitcode => {
410-
modules_config.emit_bc = true;
411-
}
412-
OutputType::LlvmAssembly => {
413-
modules_config.emit_ir = true;
414-
}
415-
OutputType::Assembly => {
416-
modules_config.emit_asm = true;
417-
}
418-
OutputType::Object => {
419-
modules_config.emit_obj = emit_obj;
420-
}
421-
OutputType::Metadata => {
422-
metadata_config.emit_obj = emit_obj;
423-
}
424-
OutputType::Exe => {
425-
modules_config.emit_obj = emit_obj;
426-
metadata_config.emit_obj = emit_obj;
427-
allocator_config.emit_obj = emit_obj;
428-
}
429-
OutputType::Mir => {}
430-
OutputType::DepInfo => {}
431-
}
432-
}
433-
434-
modules_config.set_flags(sess, no_builtins);
435-
metadata_config.set_flags(sess, no_builtins);
436-
allocator_config.set_flags(sess, no_builtins);
437-
438-
// Exclude metadata and allocator modules from time_passes output, since
439-
// they throw off the "LLVM passes" measurement.
440-
metadata_config.time_module = false;
441-
allocator_config.time_module = false;
424+
let modules_config = ModuleConfig::new(ModuleKind::Regular, sess, no_builtins);
425+
let metadata_config = ModuleConfig::new(ModuleKind::Metadata, sess, no_builtins);
426+
let allocator_config = ModuleConfig::new(ModuleKind::Allocator, sess, no_builtins);
442427

443428
let (shared_emitter, shared_emitter_main) = SharedEmitter::new();
444429
let (codegen_worker_send, codegen_worker_receive) = channel();

0 commit comments

Comments
 (0)