On Fri, Dec 04, 2020 at 09:13:50AM -0800, Linus Torvalds wrote: > On Fri, Dec 4, 2020 at 8:34 AM Luc Van Oostenryck > <luc.vanoostenryck@xxxxxxxxx> wrote: > > > > A lot of the false 'context imbalance' warnings are caused by > > a potential jump-threading being blocked between 2 conditional > > branches on the same condition because the second CBR belong > > to a non-empty BB. Often the offending instructions can be moved > > to some other BB, sometimes even with some added advantages. > > I hope you don't just use the context imbalance as a sign of this > being a good idea, because a lot of the context imbalances are likely > real and valid. Not exactly, but I confess that currently I'm focusing a lot on the '*false* context imbalance' (those I can see in the C code or in the IR that should be OK but sparse warning on them nevertheless). I've not a very clear idea of how much of those warnings are real (but I'm begin to think more and more than most are real). By far, the most common cause of these false warnings is a CBR-CBR on the same condition that is not simplified away because the second one is not empty. > I do think moving the instruction to the (single) user sounds like a > good idea in some cases, but I'm a bit worried about doing it quite > this mindlessly. It can expand on liveness a lot - while the liveness > of the result of the sunk instruction shrinks, the liveness of the > sources to the sunk instruction can grow a lot. > > That's obviously a non-issue for the use of sparse as an analysis tool > (and that's clearly the primary use), but I'd still like to think that > code generation might matter. Yes, I agree for both points. > So I think this might be better with more heuristics. And explaining > them. Right now you have one heuristic: you only sink instructions > from bb's that end with a conditional branch. I'm not entirely sure > that I understand the reason for that heuristic, it smells a bit > arbitrary to me (I suspect it was the case you saw when looking at > examples). Yes, indeed, it's arbitrary because it's the only case I'm interested about for these 'false context imbalance'. But no worries, as explained in the patch description, it's not my intention to merge this, certainly not as-is. It's more a kind of experiment to play with, a kind of exploratory tool. > On that note: would also be lovely to actually see examples of what > this results in - and not necessarily about just the context imbalance > again. Yes, I'll add this (but I'm not sure to have anything significant not related to emptying a BB ending with a CBR). > There might be cases where instruction sinking makes sense even > outside the "can we empty this bb entirely" issue. Not that I can > think of any, but I wonder if this could be used to actually shrink > liveness regions (if both the inputs to the sunk instruction are live > _anyway_ at the target, then sinking the instruction should actually > improve liveness in general, for example). I don't think I understand this. In the case of an UNOP, sinking it increase the liveness and decrease the liveness of the output, so it should not matter much. In the case of an BINOP or select, sinking it will decrease the liveness of the unique output but increase the liveness of the inputs. So, it seems to me that sinking would globally increase the liveness (contrary to moving up instructions). Am I missing something? -- Luc
