[LoopUnroll] Don't add metadata to unrolled remainder loop

[samparker]

Summary

Debug information was being corrupted when the runtime remainder loop was fully unrolled. But in the case when the remainder loop is going to be fully unrolled, there isn't the need to add loop metadata to it.

[aprantl]

test/DebugInfo/unrolled-loop-remainder.ll

6

Can you adda comment explaining what is being checked for in this test?

52

This looks dangerous. Doesn't this match almost every line?

58

Can you strip out all non-essential attributes (i.e. usually everything in quotes)?

[samparker]

Hi Adrian,

Thanks for your feedback. I was always a bit worried that I was treating a symptom and not the cause here. When I remove the target features as you suggested, the issue that I was originally observing failed to happen. I will have to dig a bit deeper.

Thanks,
sam

lib/Transforms/Utils/LoopUnrollRuntime.cpp

           const MDString *S = dyn_cast<MDString>(MD->getOperand(0));
           IsUnrollMetadata = S && S->getString().startswith("llvm.loop.unroll.");
         }
         if (!IsUnrollMetadata)
           MDs.push_back(LoopID->getOperand(i));
       }
     }
 
-    LLVMContext &Context = NewLoop->getHeader()->getContext();
+    // Only add loop metadata if the loop is not going to be completely
+    // unrolled.
     if (!UnrollRemainder) {
+      LLVMContext &Context = NewLoop->getHeader()->getContext();
       SmallVector<Metadata *, 1> DisableOperands;
       DisableOperands.push_back(MDString::get(Context,
                                               "llvm.loop.unroll.disable"));
       MDNode *DisableNode = MDNode::get(Context, DisableOperands);
       MDs.push_back(DisableNode);
-    }
 
       MDNode *NewLoopID = MDNode::get(Context, MDs);
       // Set operand 0 to refer to the loop id itself.
       NewLoopID->replaceOperandWith(0, NewLoopID);
       NewLoop->setLoopID(NewLoopID);
+    }
     return NewLoop;
   }
   else
     return nullptr;
 }
 
 /// Returns true if we can safely unroll a multi-exit/exiting loop. OtherExits
 /// is populated with all the loop exit blocks other than the LatchExit block.

                                       bool UseEpilogRemainder,
                                       bool UnrollRemainder,
                                       LoopInfo *LI, ScalarEvolution *SE,
                                       DominatorTree *DT, AssumptionCache *AC,
                                       OptimizationRemarkEmitter *ORE,
                                       bool PreserveLCSSA) {
   DEBUG(dbgs() << "Trying runtime unrolling on Loop: \n");
   DEBUG(L->dump());
+  DEBUG(UseEpilogRemainder ? dbgs() << "Using epilog remainder.\n" :
+        dbgs() << "Using prolog remainder.\n");
 
   // Make sure the loop is in canonical form.
   if (!L->isLoopSimplifyForm()) {
     DEBUG(dbgs() << "Not in simplify form!\n");
     return false;
   }
 
   // Guaranteed by LoopSimplifyForm.

     formDedicatedExitBlocks(L, DT, LI, PreserveLCSSA);
     // Generate dedicated exit blocks for the remainder loop if one exists, to
     // preserve LoopSimplifyForm.
     if (remainderLoop)
       formDedicatedExitBlocks(remainderLoop, DT, LI, PreserveLCSSA);
   }
 
   if (remainderLoop && UnrollRemainder) {
+    DEBUG(dbgs() << "Unrolling remainder loop\n");
     UnrollLoop(remainderLoop, /*Count*/Count - 1, /*TripCount*/Count - 1,
                /*Force*/false, /*AllowRuntime*/false,
                /*AllowExpensiveTripCount*/false, /*PreserveCondBr*/true,
                /*PreserveOnlyFirst*/false, /*TripMultiple*/1,
                /*PeelCount*/0, /*UnrollRemainder*/false, LI, SE, DT, AC, ORE,
                PreserveLCSSA);
   }
 
   NumRuntimeUnrolled++;
   return true;
 }

test/DebugInfo/unrolled-loop-remainder.ll

+; RUN: opt -loop-unroll -unroll-count=4 -unroll-remainder -S %s -o - | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@b = common local_unnamed_addr global i32 0, align 4, !dbg !0
+@a = common local_unnamed_addr global i32* null, align 8, !dbg !6
+
+; CHECK-LABEL: c
+; CHECK-LABEL: for.body.lr.ph:
+; CHECK: br i1 %[[CMP0:.*]], label %[[PRE:.*]], label %for.body.prol.loopexit, !dbg !24
+; CHECK-LABEL: for.body:
+; CHECK: br i1 %[[CMP1:.*]], label %[[CRIT_EDGE:.*]], label %for.body, !dbg !24, !llvm.loop !30
+; CHECK-LABEL: for.cond.for.end_crit_edge:
+; CHECK: br label %for.end, !dbg !24
+; CHECK-LABEL: for.body.prol.1:
+; CHECK: br i1 %[[CMP2:.*]], label %for.body.prol.2, label %[[EXIT:.*]], !dbg !24
+; CHECK-LABEL: for.body.prol.2:
+define i32 @c() local_unnamed_addr #0 !dbg !14 {
+entry:
+  %.pr = load i32, i32* @b, align 4, !dbg !17, !tbaa !20
+  %tobool1 = icmp eq i32 %.pr, 0, !dbg !24
+  br i1 %tobool1, label %for.end, label %for.body.lr.ph, !dbg !24
+
+for.body.lr.ph:
+  %a.promoted = load i32*, i32** @a, align 8, !dbg !25, !tbaa !26
+  %0 = sub i32 -2, %.pr, !dbg !24
+  %1 = and i32 %0, -2, !dbg !24
+  %2 = add i32 %.pr, %1, !dbg !24
+  br label %for.body, !dbg !24
+
+for.body:
+  %3 = phi i32* [ %a.promoted, %for.body.lr.ph ], [ %6, %for.body ], !dbg !28
+  %4 = phi i32 [ %.pr, %for.body.lr.ph ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %3, i64 1, !dbg !28
+  %5 = load i32, i32* %arrayidx, align 4, !dbg !28, !tbaa !20
+  %conv = sext i32 %5 to i64, !dbg !28
+  %6 = inttoptr i64 %conv to i32*, !dbg !28
+  %add = add nsw i32 %4, 2, !dbg !29
+  %tobool = icmp eq i32 %add, 0, !dbg !24
+  br i1 %tobool, label %for.cond.for.end_crit_edge, label %for.body, !dbg !24, !llvm.loop !30
+
+for.cond.for.end_crit_edge:
+  %7 = add i32 %2, 2, !dbg !24
+  store i32* %6, i32** @a, align 8, !dbg !25, !tbaa !26
+  store i32 %7, i32* @b, align 4, !dbg !32, !tbaa !20
+  br label %for.end, !dbg !24
+
+for.end:
+  ret i32 undef, !dbg !33
+}
+
+; CHECK-LABEL: d
+define void @d() local_unnamed_addr #1 !dbg !34 {
+entry:
+  ret void, !dbg !37
+}
+
+attributes #0 = { norecurse nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { norecurse nounwind readnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11, !12}
+!llvm.ident = !{!13}
+
+!0 = !DIGlobalVariableExpression(var: !1)
+!1 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 2, type: !9, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 6.0.0 (http://llvm.org/git/clang.git 044091b728654e62444a7ea10e6efb489c705bed) (http://llvm.org/git/llvm.git 1c7b55afdb1c5791e0557d9e32e2dd07c7acb2b0)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "loop.c", directory: "/work/projects/src/tests/unroll-debug-info")
+!4 = !{}
+!5 = !{!6, !0}
+!6 = !DIGlobalVariableExpression(var: !7)
+!7 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true)
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !{i32 2, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 4}
+!13 = !{!"clang version 6.0.0 (http://llvm.org/git/clang.git 044091b728654e62444a7ea10e6efb489c705bed) (http://llvm.org/git/llvm.git 1c7b55afdb1c5791e0557d9e32e2dd07c7acb2b0)"}
+!14 = distinct !DISubprogram(name: "c", scope: !3, file: !3, line: 3, type: !15, isLocal: false, isDefinition: true, scopeLine: 3, isOptimized: true, unit: !2, variables: !4)
+!15 = !DISubroutineType(types: !16)
+!16 = !{!9}
+!17 = !DILocation(line: 4, column: 12, scope: !18)
+!18 = distinct !DILexicalBlock(scope: !19, file: !3, line: 4, column: 5)
+!19 = distinct !DILexicalBlock(scope: !14, file: !3, line: 4, column: 5)
+!20 = !{!21, !21, i64 0}
+!21 = !{!"int", !22, i64 0}
+!22 = !{!"omnipotent char", !23, i64 0}
+!23 = !{!"Simple C/C++ TBAA"}
+!24 = !DILocation(line: 4, column: 5, scope: !19)
+!25 = !DILocation(line: 5, column: 13, scope: !18)
+!26 = !{!27, !27, i64 0}
+!27 = !{!"any pointer", !22, i64 0}
+!28 = !DILocation(line: 5, column: 15, scope: !18)
+!29 = !DILocation(line: 4, column: 21, scope: !18)
+!30 = distinct !{!30, !24, !31}
+!31 = !DILocation(line: 5, column: 18, scope: !19)
+!32 = !DILocation(line: 4, column: 17, scope: !18)
+!33 = !DILocation(line: 6, column: 1, scope: !14)
+!34 = distinct !DISubprogram(name: "d", scope: !3, file: !3, line: 7, type: !35, isLocal: false, isDefinition: true, scopeLine: 7, isOptimized: true, unit: !2, variables: !4)
+!35 = !DISubroutineType(types: !36)
+!36 = !{null}
+!37 = !DILocation(line: 7, column: 11, scope: !34)

test/Transforms/LoopUnroll/runtime-unroll-remainder.ll

 ; CHECK: [[MOD:%[a-z.0-9]+]] = icmp eq i64 %xtraiter, 0
 ; CHECK: br i1 [[MOD]], label %[[EXIT:.*]], label %[[EPIL_PEEL0_PRE:.*]]
 
 ; CHECK: [[EPIL_PEEL0_PRE]]:
 ; CHECK: br label %[[EPIL_PEEL0:.*]]
 
 ; CHECK: [[EPIL_PEEL0]]:
 ; CHECK: [[PEEL_CMP0:%[a-z.0-9]+]] = icmp eq i64 %xtraiter, 1
-; CHECK: br i1 [[PEEL_CMP0]], label %[[EPIL_EXIT:.*]], label %[[EPIL_PEEL1:.*]],
+; CHECK: br i1 [[PEEL_CMP0]], label %[[EPIL_EXIT:.*]], label %[[EPIL_PEEL1:.*]]
 
 ; CHECK: [[EPIL_EXIT]]:
 ; CHECK: br label %[[EXIT]]
 
 ; CHECK: [[EXIT]]:
 ; CHECK: ret i32
 
 ; CHECK-LABEL: for.body:
 ; CHECK: [[INDVAR0:%[a-z.0-9]+]] = phi i64 [ 0, %for.body.lr.ph
 ; CHECK: [[ITER:%[a-z.0-9]+]] = phi i64 [ %unroll_iter
 ; CHECK: or i64 [[INDVAR0]], 1
 ; CHECK: or i64 [[INDVAR0]], 2
 ; CHECK: or i64 [[INDVAR0]], 3
 ; CHECK: add nsw i64 [[INDVAR0]], 4
 ; CHECK: [[SUB:%[a-z.0-9]+]] = add i64 [[ITER]], -4
 ; CHECK: [[ITER_CMP:%[a-z.0-9]+]] = icmp eq i64 [[SUB]], 0
 ; CHECK: br i1 [[ITER_CMP]], label %[[LOOP_EXIT:.*]], label %for.body
 
 ; CHECK: [[EPIL_PEEL1]]:
 ; CHECK: [[PEEL_CMP1:%[a-z.0-9]+]] = icmp eq i64 %xtraiter, 2
-; CHECK: br i1 [[PEEL_CMP1]], label %[[EPIL_EXIT]], label %[[EPIL_PEEL2:.*]],
+; CHECK: br i1 [[PEEL_CMP1]], label %[[EPIL_EXIT]], label %[[EPIL_PEEL2:.*]]
 
 ; CHECK: [[EPIL_PEEL2]]:
 ; CHECK: br label %[[EXIT]]
 
 for.body:
   %indvars.iv = phi i64 [ 0, %for.body.lr.ph ], [ %indvars.iv.next, %for.body ]
   %c.010 = phi i32 [ 0, %for.body.lr.ph ], [ %add, %for.body ]
   %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv