[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D85710#2212174 <https://reviews.llvm.org/D85710#2212174>, @rjmccall wrote:

> I always imagined that we'd implement this by just inverting the emission: emit the RHS of the assignment and then pass a callback down to a function that descended into conditional operators and called the callback when it hit a leaf.  Having an LValue case that's an arbitrarily-nested conditional seems unfortunate.

I thought I'd convinced myself that that approach couldn't work, but I'm no longer so convinced. (I had thought there were cases where we would need to run arbitrary code between creating the lvalue and disposing of it, and that arbitrary code could be something we were only able to emit once for whatever reason.) The fact that assignment and compound-assignment operators all evaluate their RHS before their LHS (in the language modes where they're sequenced) helps a lot; expressions such as `((cond ? a.x : a.y) += f()) *= g()` still seem like they could compositionally work with this model, and even things like `(cond1 ? b : (cond2 ? a.x : a.y) += f()) += g()`. Hm, but what about `int n = (cond ? a.x : a.y) += 1;`? I guess we'd need to carry the intent to perform a load of the conditional lvalue into the callback too.

OK, I think that all works, but I'm a lot less confident of the absence of surprising corners that would defeat that strategy than with the approach of this patch -- the correctness of doing this with a single branch seems to hinge crucially on subtle program structure restrictions, in particular the presumptive fact that we never need control flow to converge between creating the lvalue and consuming it. On the other hand, it would also let us emit better IR for things like `(a += 1) *= 2` (removing the intermediate store to `a`) if we wanted, and maybe it's a good strategy to pursue for that reason?

I can give that a go next time I find some cycles to work on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85710