Conversation
eddyb
left a comment
eddyb
left a comment
There was a problem hiding this comment.
LGTM, approving for when you get windows working, the comments I left could probably turn into issues?
| fn set_span(&mut self, span: Span) { | ||
| self.current_span = Some(span); | ||
| let loc = self.cx.tcx.sess.source_map().lookup_char_pos(span.lo()); | ||
| let file = self.builder.def_string(format!("{}", loc.file.name)); |
There was a problem hiding this comment.
Heh, we should cache this (keying on Rc<SourceFile> might work, but idk if it =='s by address).
EDIT: oh, I see, def_string already caches by string. I was imagining something more aggressive but this is probably fine for now.
| let loc = self.cx.tcx.sess.source_map().lookup_char_pos(span.lo()); | ||
| let file = self.builder.def_string(format!("{}", loc.file.name)); | ||
| self.emit() | ||
| .line(file, loc.line as u32, loc.col_display as u32); |
There was a problem hiding this comment.
I wonder if we should track the fact that we're not using the debuginfo infrastructure for this.
On the one hand, we should integrate it and eventually support a lot of debuginfo through extensions like NonSemantic.Shader.DebugInfo.100.
OTOH, we do need to always annotate certain things (probably most instructions tbh, given how many of them can fail validation) if we want to have a chance at nice error reporting.
Definitely fine with it landing as-is FWIW.
| if i + 1 < block.instructions.len() | ||
| && block.instructions[i].class.opcode == Op::Line | ||
| && block.instructions[i + 1].class.opcode == Op::Line | ||
| { | ||
| block.instructions.remove(i); |
There was a problem hiding this comment.
Oof, this is pretty much dedup_by, but we need to keep the last not first in a group of OpLines.
I don't know if it's going to be more efficient but if you did want to use dedup_by you'd have to reverse the entire block.instructions, dedup_by, then reverse it again, ughhh.
There was a problem hiding this comment.
thinking about this did make me realize that reversing a list is actually pretty much the same number of reads/writes as removing index 0 of a list, neat, that's a little unintuitive, haha.
Anyway, uh, dedup_by passes in the arguments as &mut, so a cursed solution that I think I'll do is mem::swap them if they're both OpLines :3
2 participants
Update dependencies (both rspirv and spirv-tools-rs) to be able to use OpLine, and do so.
The
remove_duplicate_linesfor loop is absolute garbage,O(n^2)terribleness, so if someone wants to hack up a clever thing, please do so~