[PATCH] Ensure SROA doesn't prematurely promote an alloca that's being revisited

[Chandler Carruth]

On Wed, Jan 8, 2014 at 3:10 PM, Evan Cheng <evan.cheng at apple.com> wrote:

> Attached is a patch (and the test case) that fix a SROA edge case. An
> alloca can be added back to the worklist to be revisited during rewrite of
> a MemTransferInst. If the alloca is added to the "promotable" queue and
> then added again when it's revisited, mem2reg will crash. The patch simply
> prevents promotion of an alloca that is scheduled to be revisited.
>

Ow! Good catch!


>
> The patch isn't ideal. It's following the
> AllocaSliceRewriter::isUsedByRewrittenSpeculatableInstructions() example by
> caching a boolean state. But it's correct and as clean a patch as I can
> come up with.
>

Hmm, yea. I hated isUsedByRewrittenSpeculatableInstructions when it went
in, but I still don't have a better solution. However, there might be a
better solution here. The speculation case is particularly hard because the
place where this is being "used" can't easily be updated. But this case
might be easier. Could you instead of this, either:

1) Check for promotability prior to adding the alloca back to the worklist
(I doubt this would work), or
2) Remove the alloca from the worklist (or skip it when popping it off of
the worklist) when it has already be slated for promotion?

I'm hopeful that we could do #2, but not confident. I feel like we really
should go ahead and promote in this case rather than carrying on and
re-processing the alloca *again* when it is already in a good state.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140108/c1aa6fd4/attachment.html>