[PATCH] R600/SI: Fix verifier errors from the SIAnnotateControlFlow pass

[Christian König]

On 30.04.2015 22:12, Tom Stellard wrote:
> This pass was generating 'Instruction does not dominate all uses!'
> errors for programs which had loops with a condition variable that
> depended on the result of a phi instruction from outside of the loop.
>
> The pass was inserting new phi nodes outside of the loop which used values
> defined inside the loop.
>
> http://bugs.freedesktop.org/show_bug.cgi?id=90056

LGTM.

> ---
>   lib/Target/R600/SIAnnotateControlFlow.cpp | 10 +++++++-
>   test/CodeGen/R600/si-annotate-cf.ll       | 38 +++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/lib/Target/R600/SIAnnotateControlFlow.cpp b/lib/Target/R600/SIAnnotateControlFlow.cpp
> index d39ab3f..ccfbf1b 100644
> --- a/lib/Target/R600/SIAnnotateControlFlow.cpp
> +++ b/lib/Target/R600/SIAnnotateControlFlow.cpp
> @@ -209,7 +209,15 @@ void SIAnnotateControlFlow::insertElse(BranchInst *Term) {
>   /// \brief Recursively handle the condition leading to a loop
>   Value *SIAnnotateControlFlow::handleLoopCondition(Value *Cond, PHINode *Broken,
>                                                     llvm::Loop *L) {
> -  if (PHINode *Phi = dyn_cast<PHINode>(Cond)) {
> +
> +  // Only search through PHI nodes which are inside the loop.  If we try this
> +  // with PHI nodes that are outside of the loop, we end up inserting new PHI
> +  // nodes outside of the loop which depend on values defined inside the loop.
> +  // This will break the module with
> +  // 'Instruction does not dominate all users!' errors.
> +  PHINode *Phi = nullptr;
> +  if ((Phi = dyn_cast<PHINode>(Cond)) && L->contains(Phi)) {
> +
>       BasicBlock *Parent = Phi->getParent();
>       PHINode *NewPhi = PHINode::Create(Int64, 0, "", &Parent->front());
>       Value *Ret = NewPhi;
> diff --git a/test/CodeGen/R600/si-annotate-cf.ll b/test/CodeGen/R600/si-annotate-cf.ll
> index 3f30988..bbcb861 100644
> --- a/test/CodeGen/R600/si-annotate-cf.ll
> +++ b/test/CodeGen/R600/si-annotate-cf.ll
> @@ -23,3 +23,41 @@ ENDLOOP:
>   ENDIF:
>     br i1 %1, label %ENDLOOP, label %ENDIF
>   }
> +
> +
> +; FUNC-LABEL: {{^}}phi_cond_outside_loop:
> +; FIXME: This could be folded into the s_or_b64 instruction
> +; SI: s_mov_b64 [[ZERO:s\[[0-9]+:[0-9]+\]]], 0
> +; SI: [[LOOP_LABEL:[A-Z0-9]+]]
> +; SI: v_cmp_ne_i32_e32 vcc, 0, v{{[0-9]+}}
> +
> +; SI_IF_BREAK instruction:
> +; SI: s_or_b64 [[BREAK:s\[[0-9]+:[0-9]+\]]], vcc, [[ZERO]]
> +
> +; SI_LOOP instruction:
> +; SI: s_andn2_b64 exec, exec, [[BREAK]]
> +; SI: s_cbranch_execnz [[LOOP_LABEL]]
> +; SI: s_endpgm
> +
> +define void @phi_cond_outside_loop(i32 %a, i32 %b) {
> +entry:
> +  %0 = icmp eq i32 %a , 0
> +  br i1 %0, label %if, label %else
> +
> +if:
> +  br label %endif
> +
> +else:
> +  %1 = icmp eq i32 %b, 0
> +  br label %endif
> +
> +endif:
> +  %2 = phi i1 [0, %if], [%1, %else]
> +  br label %loop
> +
> +loop:
> +  br i1 %2, label %exit, label %loop
> +
> +exit:
> +  ret void
> +}