[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

In D105516#2892512 <https://reviews.llvm.org/D105516#2892512>, @rjmccall wrote:

> I agree with Eli: we should decide what the goals are here and then use those goals to decide if we can identify a desirable permanent feature and, if so, what the appropriate name for that feature is.
>
> It sounds like your goal is to get readable assembly that still corresponds fairly literally to your original code, in the sense that the readability of -O0 assembly is often undermined by the sheer amount of code and all the extra, unnecessary work it seems to do.  However, I've found that a lot of the extra -O0 code is not actually from loads and stores to local variables, it's from the -O0 instruction selection and register allocation, which often combine to do very silly things.  Have you looked into whether you get more readable code by just running normal -O0 IR through the non-O0 codegen pipeline?  Because the problem with doing just mem2reg is that that's already a fairly major non-literal change to the code, and at some point it's tricky to say what exactly should be part of this new pipeline; whereas still emitting exactly what the abstract machine says to do, just with less nonsense from fast-isel, is a lot easier to define.

Well, I'm dealing with RISC-V which doesn't have FastISel. And no, running -O0 IR through a different pipeline is never going to work well without mem2reg (and you can't mem2reg -O0 IR without -disable-O0-optnone), so you need at least this to work well. Whether or not there's a useful load of additional stuff you could do, maybe, but I do think this patch in and of itself makes a huge difference, and the limited scope of it is beneficial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105516