Repository navigation
Conversation
|
cc @bjorn3 |
rustbot
added
A-LLVM
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Cc @nikic |
| ) -> TargetMachineFactoryFn<Self> { | ||
| back::write::target_machine_factory(sess, optlvl) | ||
| fn target_machine_factory(&self, sess: &Session) -> TargetMachineFactoryFn<Self> { | ||
| back::write::target_machine_factory(sess, sess.opts.optimize) |
There was a problem hiding this comment.
Can this be changed to only accept sess?
There was a problem hiding this comment.
I was considering that, but there is a "create_informational_machine" (or thereabouts) function that passes OptLevel::No, and I was unsure whether passing the opt level there would cause perf regressions.
There was a problem hiding this comment.
That function is only used for fetching information from LLVM, not for any actual compilation, so I think it would't affect performance at all, but I don't know if LLVM does some work on creating the optimization pipeline in advance when creating a target machine.
There was a problem hiding this comment.
Yep, that's exactly my doubt too :-)
Turns out that all uses were effectively equivalent to using sess.opts.optimize.
04c5c5d to
43aa445
Compare
|
☔ The latest upstream changes (presumably #165597) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
Turns out that all uses were effectively equivalent to using sess.opts.optimize.
In particular, the output of that function was only ever passed to target_machine_factory, which ignores it on anything but llvm, and on llvm it only passed it to to_llvm_opt_settings, where the difference between Size/SizeMin/More is only the second return value, which is immediately dropped. So replacing all uses of backend_optimization_level with sess.opts.optimize is effectively equivalent.
LLM usage: I asked $LLM to review the commit, and it didn't complain about anything. Also $LLM initially figured out (and I confirmed) that backend_optimization_level was effectively a no-op.