[PATCH] D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies

Fri Mar 8 02:49:06 PST 2019

jmorse requested review of this revision.
jmorse added a comment.

In D58238#1422261 <https://reviews.llvm.org/D58238#1422261>, @bjope wrote:

> Yes, I was just thinking out loud about sinking/scheduling in general and not so much the specific situation with MachineSink. But I also wondered if the sinking done by MachineSink could be seen as a special case of sinking in general, where we sink one instruction at a time. For MachineSink we eventually reach the end of the BB, and then continue into a successor. So how do we determine when it is time to insert "undef" when sinking one instruction at a time? What kind of reorderings should/shouldn't trigger that we insert an "undef"? I guess this is one thing we should try to describe in the documentation (also for DGB_VALUE), to make sure new developers understand the basic logic behind how we implement these things.

Ahhh, now that all fits in my mind, cool. I'll certainly ship a docs patch, when I've convinced myself I know what's going on at the CodeGen level :o

> I only had a quick look at D59027 <https://reviews.llvm.org/D59027>, but it feels like it tries to handle this a little bit more "carefully" (avoiding <optimized out> in some situations compared to this patch). 
>  Btw, I'll be OOO for a week. So I won't be able to give much more feedback right now. No need to wait for my approval if you get LGTM from someone else while I'm away.

Cool, now clicking a phab button that allegedly will put this back in for review.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58238/new/

https://reviews.llvm.org/D58238