@oli @Wesley Wiser hey, do you think it would be cheap to check in the constant folder if a local always has a constant value whenever initialized?
I guess it could be its own pass post-constant-folding, that checks for one initialization and no mutation
I want to have
debug x => _123; replaced with e.g.
debug x => const 42usize; wherever possible
the other thing we could do is
debug a => &b.c; (value of debug var a is always the address of a place with no runtime indexing or derefs)
the latter wouldn't count as a borrow because only the debugger would see it, and also both it and the constant form could use
the goal is to reduce the impact of debuginfo on optimizations and codegen
but maybe I should do some statistics on the kinds of things that are e.g. unnecessarily on the stack in debug mode
@eddyb By "constant folder" do you mean const-prop or something else?
yeah I guess it's called const-prop
I'd argue propagation is just replacing uses with the value at the definition, whereas folding is e.g.
_5 = 1 + 2 =>
_5 = 3 (there's no propagation there)
the current pass is a combination of propagation and folding
I can't imagine it would be expensive
We've already done that check and know what locals satisfy that constraint
oh right you're not relying on dataflow?
I guess you couldn't have, @ecstatic-morse's stuff is pretty recent
Nah, not yet
@eddyb I took a stab at implementing this. Does this seem about right to you? https://github.com/rust-lang/rust/compare/master...wesleywiser:consts_in_debuginfo
quick side note just from looking at the tests: use
Display formatting for the constant instead of
Thanks! Will do
Yeah, that's way better:
- debug x => _1; // in scope 1 at $DIR/scalar_literal_propagation.rs:3:9: 3:10 + debug x => const 1u32; // in scope 1 at $DIR/scalar_literal_propagation.rs:3:9: 3:10
I wouldn't change
it refers to the LHS not the RHS even if it's computed from the RHS.... it's kind of messed up
PerLocalVarDebugInfo also doesn't seem necessary
Ah yeah I think I only matched against it here so that's not necessary https://github.com/rust-lang/rust/compare/master...wesleywiser:consts_in_debuginfo#diff-59edf51044167945d3d123c17e706bfdR544
so hmm starting from the MIR
I wouldn't split the
VarDebugInfoValue, although it doesn't feel great either. I wonder what I used in the SROA PR
Is that in your SROA PR?
there it's used for simple leaves vs composite (each field has its own
@Wesley Wiser so, the reason changing
PerLocalVarDebugInfo doesn't make sense to me is that it's specifically the information you need to emit debuginfo for initializing a given
if it's a constant there's no local involved
The reason I did that was I needed all the same fields except
you don't need to gather them into a
Which is not a great reason to hack it in there lol
you don't need "var name" for constants
since they can't have it anyway (only fn args and instructions can have names)
so you should just get away with spill +
oh I see, you're triggering it from
okay so uhhh my advice to keep this simple likely requires renaming
I did it that way because the current system seems pretty oriented around locals which we've removed for constants.
and instead of
per_local[place.local].push( it can do
well it's oriented around locals in a very specific way that makes no sense for constants
specifically it's on-demand for locals
or on-assignment rather
Ok, I think I'm following that.
no need to do that kind of deferral for constants
And yeah if we do it that way I don't need to collect to a vec or modify PerLocalDebugInfo
thanks for pointing me to this btw
Thanks, that's really helpful!
as you may have noticed I've been kind of out of it
where "it" is everything
I knew you'd probably be interested :slight_smile:
Re the SROA changes. Do you still think I should try to incorporate those into this?
just the name :P
i.e. value -> contents
only if you think it makes sense
I forget exactly what my line of reasoning was
That's totally fine
I have no strong opinions here lol
Thanks for taking a look at this! I really appreciate it