[PATCH] D59129: [SROA] WIP: Lowering alloca is not always beneficial

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

> I was sort of thinking you could write the reverse transform in MemCpyOpt, since it does a similar sorts of analysis, but maybe we'd have to do it later, or patch SROA anyway, to avoid two transforms fighting each other.

Ah, yes, MemCpyOpt. I haven't looked in great detail at that because it hasn't bothered me yet. That is, in experiments when I completely disabled SROA, I didn't find MemCpyOpt reversing things. I am guessing because the analysis and transformation is far from straightforward. To me, it sounds easier to restrict the rewrite/transformation here in SROA, rather than trying to recreate the IR in MemCpyOpt that we already once had. But again, I haven't looked much into MemCpyOpt, so if you think it would be better there, I will start looking into that.

>> If you look at shouldExpand, you see that a decision is made per alloca slice
> 
> You're returning early from SROA::runOnAlloca. That's a decision on the whole alloca: if there's an expensive memcpy into any part of the alloca, don't do any splitting on the whole alloca. The alternative is to make the decision on a per-slice level: given a subset of slices that are the target of a memcpy, "merge" those slices so splitAlloca() doesn't split them away from each other.

Ah yes, you're right; I wanted to do it per alloca slice, but that's currently not the case. I can't remember if I had good reasons for that, and will double check what the codegen differences are.


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

https://reviews.llvm.org/D59129