Page MenuHomePhabricator

[CGP] Ensure sinking multiple instructions does not invalidate dominance checks
ClosedPublic

Authored by dmgreen on Sep 9 2019, 1:12 PM.

Details

Summary

In MVE, as of rL371218, we are attempting to sink chains of instructions such as:

%l1 = insertelement <8 x i8> undef, i8 %l0, i32 0
%broadcast.splat26 = shufflevector <8 x i8> %l1, <8 x i8> undef, <8 x i32> zeroinitializer

In certain situations though, we can end up breaking the dominance relations of instructions. This happens when we sink the instruction into a loop, but cannot remove the originals. The Use is updated, which might in fact be a Use from the second instruction to the first.

This attempts to fix that by reversing the order of instruction that are sunk, and ensuring that we update the uses on new instructions if they have already been sunk, not the old ones.

Diff Detail

Repository
rL LLVM

Event Timeline

dmgreen created this revision.Sep 9 2019, 1:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2019, 1:12 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
samparker added inline comments.Sep 10 2019, 2:36 AM
llvm/lib/CodeGen/CodeGenPrepare.cpp
6206 ↗(On Diff #219416)

So it's not possible for an instruction to be inserted multiple times?

dmgreen marked an inline comment as done.Sep 10 2019, 2:54 PM
dmgreen added inline comments.
llvm/lib/CodeGen/CodeGenPrepare.cpp
6206 ↗(On Diff #219416)

Hmm. Yeah. Sinking the same instruction form multiple uses? It would be cloned more than once. I don't think anything currently uses it...

We do need to make sure they are deleted in a deterministic order. I can change it to a SetVector. Good suggestion, thanks!

dmgreen updated this revision to Diff 219611.Sep 10 2019, 2:55 PM
This revision is now accepted and ready to land.Sep 12 2019, 12:12 AM
This revision was automatically updated to reflect the committed changes.