[PATCH] D48721: Patch to fix pragma metadata for do-while loops

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D48721#1150333, @bjope wrote:

> I tried running
>
>   /clang -cc1 -O3 -funroll-loops -S -emit-llvm pragma-do-while-unroll.cpp -o - -mllvm -print-after-all
>  
>
>
> and I get this
>
>   ...
>   !2 = distinct !{!2, !3}
>   !3 = !{!"llvm.loop.unroll.count", i32 3}
>   !4 = !{!5, !5, i64 0}
>   !5 = !{!"int", !6, i64 0}
>   !6 = !{!"omnipotent char", !7, i64 0}
>   !7 = !{!"Simple C/C++ TBAA"}
>   !8 = distinct !{!8, !9}
>   !9 = !{!"llvm.loop.unroll.count", i32 5}
>   *** IR Dump After Combine redundant instructions ***
>   ; Function Attrs: nounwind
>   define i32 @test(i32* %a, i32 %n) local_unnamed_addr #0 {
>   entry:
>     br label %do.body, !llvm.loop !2
>  
>   do.body:                                          ; preds = %do.body, %entry
>     %i.0 = phi i32 [ 0, %entry ], [ %inc, %do.body ]
>     %sum.0 = phi i32 [ 0, %entry ], [ %add5, %do.body ]
>     %0 = zext i32 %i.0 to i64
>     %arrayidx = getelementptr inbounds i32, i32* %a, i64 %0
>     %1 = load i32, i32* %arrayidx, align 4, !tbaa !4
>     %add = add nsw i32 %1, 1
>     store i32 %add, i32* %arrayidx, align 4, !tbaa !4
>     %add5 = add nsw i32 %sum.0, %add
>     %inc = add nuw nsw i32 %i.0, 1
>     %cmp = icmp slt i32 %inc, %n
>     br i1 %cmp, label %do.body, label %do.end, !llvm.loop !2
>  
>   do.end:                                           ; preds = %do.body
>     br label %do.body6, !llvm.loop !8
>  
>   do.body6:                                         ; preds = %do.body6, %do.end
>     %i.1 = phi i32 [ 0, %do.end ], [ %inc15, %do.body6 ]
>     %sum.1 = phi i32 [ %add5, %do.end ], [ %add14, %do.body6 ]
>     %2 = zext i32 %i.1 to i64
>     %arrayidx8 = getelementptr inbounds i32, i32* %a, i64 %2
>     %3 = load i32, i32* %arrayidx8, align 4, !tbaa !4
>     %add9 = add nsw i32 %3, 1
>     store i32 %add9, i32* %arrayidx8, align 4, !tbaa !4
>     %add14 = add nsw i32 %sum.1, %add9
>     %inc15 = add nuw nsw i32 %i.1, 1
>     %cmp17 = icmp slt i32 %inc15, %n
>     br i1 %cmp17, label %do.body6, label %do.end18, !llvm.loop !8
>  
>   do.end18:                                         ; preds = %do.body6
>     ret i32 %add14
>   }
>   *** IR Dump After Simplify the CFG ***
>   ; Function Attrs: nounwind
>   define i32 @test(i32* %a, i32 %n) local_unnamed_addr #0 {
>   entry:
>     br label %do.body, !llvm.loop !2
>  
>   do.body:                                          ; preds = %do.body, %entry
>     %i.0 = phi i32 [ 0, %entry ], [ %inc, %do.body ]
>     %sum.0 = phi i32 [ 0, %entry ], [ %add5, %do.body ]
>     %0 = zext i32 %i.0 to i64
>     %arrayidx = getelementptr inbounds i32, i32* %a, i64 %0
>     %1 = load i32, i32* %arrayidx, align 4, !tbaa !4
>     %add = add nsw i32 %1, 1
>     store i32 %add, i32* %arrayidx, align 4, !tbaa !4
>     %add5 = add nsw i32 %sum.0, %add
>     %inc = add nuw nsw i32 %i.0, 1
>     %cmp = icmp slt i32 %inc, %n
>     br i1 %cmp, label %do.body, label %do.body6, !llvm.loop !8
>  
>   do.body6:                                         ; preds = %do.body, %do.body6
>     %i.1 = phi i32 [ %inc15, %do.body6 ], [ 0, %do.body ]
>     %sum.1 = phi i32 [ %add14, %do.body6 ], [ %add5, %do.body ]
>     %2 = zext i32 %i.1 to i64
>     %arrayidx8 = getelementptr inbounds i32, i32* %a, i64 %2
>     %3 = load i32, i32* %arrayidx8, align 4, !tbaa !4
>     %add9 = add nsw i32 %3, 1
>     store i32 %add9, i32* %arrayidx8, align 4, !tbaa !4
>     %add14 = add nsw i32 %sum.1, %add9
>     %inc15 = add nuw nsw i32 %i.1, 1
>     %cmp17 = icmp slt i32 %inc15, %n
>     br i1 %cmp17, label %do.body6, label %do.end18, !llvm.loop !8
>  
>   do.end18:                                         ; preds = %do.body6
>     ret i32 %add14
>   }
>   ...
>
>
> So up until simplifyCFG I think things look pretty good with different loop-metadata for the two loops. But when simplifyCFG is tranforming
>
>     br i1 %cmp, label %do.body, label %do.end, !llvm.loop !2
>  
>   do.end:                                           ; preds = %do.body
>     br label %do.body6, !llvm.loop !8
>
>
> into
>
>   br i1 %cmp, label %do.body, label %do.body6, !llvm.loop !8
>   
>
> we get incorrect metadata for one branch target.
>
> Is the fault that the metadata only should be put on the back edge, not the branch in the preheader?


Yea. Our past thinking has been that any backedge in the loop is valid. The metadata shouldn't end up other places, although it's benign unless those other places are (or may later become) a backedge for some different loop.


Repository:
  rC Clang

https://reviews.llvm.org/D48721