On Mon, Aug 24, 2026 at 10:28 PM Robin Dapp <[email protected]> wrote: > > > There's undistribute_ops_list which does the reverse of "expanding" > > (hardwired to plus/mult). This handles non-constant factors just fine, > > so I wonder why you want to restrict expanding to constant factors? > > Should I be reading this as "the approach could be fine but rather add > non-constant factors" or "rather put it in undistribute_ops_list"? :)
The former. > The latter is what I started out with but it's restricted to depth 1 +,* > trees. I decided against non-constant factors in the former because: > (1) undistribute_ops_list would revert stuff. But that's OK? The goal is to allow simplification inbetween both? > (2) non-constant factors complicate everything and converting > a * b + c * b > into > (a + c) * b > would introduce code-size and perf regression, depending e.g. on > how CSE'd the tree already is. At some point I gave up. Hmm, I see (and sure, reassoc on its own fights with CSE). > (3) it allows easier costing and overflow checks. > > Generally, do you see a way forward for this "preprocessing" step? Oh, I was looking for a rationale for the restriction to constant-ness (and specifically - off my head, without rechecking the patch - data structure limitations from that decision). So yes, we can restrict to constants, but can we keep the infrastructure in a way that we _could_ allow non-constants? Like maybe SSA default defs (parameter values)? > > I do like to see this independently. I'll note that we have to be > > careful > > to not throw away all UB overflow knowledge without good reason. If > > we consider CSE and (a + b) + c vs. a + (b + c) then with unsigned > > arithmetic we achieve that by reassoc canonicalizing the summation > > order. For signed arithmetic we'd like to do the same, but we have to > > realize that the associated sequence is already computed at a > > dominating place to avoid rewriting the sequence into unsigned. > > Ok, it's possible to split out that patch. I even started with it > before the rest but the costing the other parts introduce makes it > easier, theoretically at least, to not unnecessarily throw away signed > operations. To add, assuming after gathering the ops list and performing some simplification, we might want to try hard to recover parts in the result that had been computed in the original IL. Consider ((a + b) + (d - c) + c) we want (a + b) + d and not, say, (int)(((unsigned)a + (unsigned)d) + (unsigned)b) Note I only looked at parts of the series, so in case you have no changes pending tell me and I'll actually review the whole thing. Otherwise I'll wait for a v2. Thanks for working on this, Richard. > > -- > Regards > Robin >
