[MemorySSA] Do not create memoryaccesses for debug info intrinsics.

[asbirlea]

Summary

Do not model debuginfo intrinsics in MemorySSA.
Regularly these are non-memory modifying instructions. With -disable-basicaa, they were being modelled as Defs.

[aprantl]

lib/Analysis/MemorySSA.cpp

1740 ↗

(On Diff #219197)

If you are not going to use a switch here, dyn_cast<DbgInfoIntrinsic>() is shorter.

[asbirlea]

Address comment.

[dblaikie]

Any chance of test coverage? (not sure how the rest of this analysis is tested (or how we test analyses in general))

[george.burgess.iv]

Thanks for this!

+1 for a simple -disable-basicaa test please.

lib/Analysis/MemorySSA.cpp

287 ↗

(On Diff #219201)

this should be dynamically dead code, no? II is sourced from MD, so there has to be an existing Def for these. if we're special-casing elsewhere to ensure these Defs are never created, ...

(if you'd rather make these new cases lead to assert(false && "debuginfo shouldn't have associated defs!"); or similar, i'm equally content with that)

1734 ↗

(On Diff #219201)

nit: please include a note saying something like debuginfo intrinsics may be considered clobbers when we have a nonstandard AA pipeline, so we special-case them

[asbirlea]

Add testcase.
Address comments.

[aprantl]

lib/Analysis/MemorySSA.cpp

292 ↗

(On Diff #219346)

This should be llvm_unreachable().

[aprantl]

test/Analysis/MemorySSA/debugvalue2.ll

2 ↗

(On Diff #219346)

Is there nothing other than checking for no assertion that we could check for in the output?

[asbirlea]

Address comments.
Updated test to check no MemoryDef exists. Added comment to leave a trail on how to reproduce the failure if debuginfo is modelled in MSSA.

[george.burgess.iv]

thanks!

lib/Analysis/MemorySSA.cpp

293 ↗

(On Diff #219574)

please remove the assert; llvm_unreachable alone should suffice.

llvm/trunk/lib/Analysis/MemorySSA.cpp

case Intrinsic::lifetime_start:
         return {false, NoAlias};
       AR = AA.alias(MemoryLocation(II->getArgOperand(1)), UseLoc);
       return {AR != NoAlias, AR};
     case Intrinsic::lifetime_end:
     case Intrinsic::invariant_start:
     case Intrinsic::invariant_end:
     case Intrinsic::assume:
       return {false, NoAlias};
+    case Intrinsic::dbg_addr:
+    case Intrinsic::dbg_declare:
+    case Intrinsic::dbg_label:
+    case Intrinsic::dbg_value:
+      llvm_unreachable("debuginfo shouldn't have associated defs!");
     default:
       break;
     }
   }
 
   if (UseCall) {
     ModRefInfo I = AA.getModRefInfo(DefInst, UseCall);
     AR = isMustSet(I) ? MustAlias : MayAlias;

 }
 
 /// Helper function to create new memory accesses
 template <typename AliasAnalysisType>
 MemoryUseOrDef *MemorySSA::createNewAccess(Instruction *I,
                                            AliasAnalysisType *AAP,
                                            const MemoryUseOrDef *Template) {
   // The assume intrinsic has a control dependency which we model by claiming
-  // that it writes arbitrarily. Ignore that fake memory dependency here.
+  // that it writes arbitrarily. Debuginfo intrinsics may be considered
+  // clobbers when we have a nonstandard AA pipeline. Ignore these fake memory
+  // dependencies here.
   // FIXME: Replace this special casing with a more accurate modelling of
   // assume's control dependency.
   if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
-    if (II->getIntrinsicID() == Intrinsic::assume)
+    if (II->getIntrinsicID() == Intrinsic::assume || isa<DbgInfoIntrinsic>(II))
       return nullptr;
 
   bool Def, Use;
   if (Template) {
     Def = dyn_cast_or_null<MemoryDef>(Template) != nullptr;
     Use = dyn_cast_or_null<MemoryUse>(Template) != nullptr;
 #if !defined(NDEBUG)
     ModRefInfo ModRef = AAP->getModRefInfo(I, None);

llvm/trunk/test/Analysis/MemorySSA/debugvalue2.ll

+; RUN: opt -disable-basicaa -print-memoryssa -disable-output %s 2>&1 | FileCheck %s
+
+; Note that the test crashes the MemorySSA verification when doing loop-rotate,
+; if debuginfo is modelled in MemorySSA, due to the fact that MemorySSA is not
+; updated when adding/removing debuginfo intrinsics.
+
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: @overflow_iter_var
+; CHECK-NOT: MemoryDef
+define void @overflow_iter_var() !dbg !11 {
+entry:
+  call void @llvm.dbg.value(metadata i16 0, metadata !16, metadata !DIExpression()), !dbg !18
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.body, %entry
+  call void @llvm.dbg.value(metadata i16 0, metadata !16, metadata !DIExpression()), !dbg !18
+  call void @llvm.dbg.value(metadata i16 undef, metadata !20, metadata !DIExpression()), !dbg !21
+  br i1 undef, label %for.end, label %for.body
+
+for.body:                                         ; preds = %for.cond
+  %0 = load i16, i16* undef, align 1
+  br label %for.cond
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, nameTableKind: None)
+!1 = !DIFile(filename: "2_loops.c", directory: "/")
+!2 = !{}
+!3 = !{}
+!6 = !DICompositeType(tag: DW_TAG_array_type, baseType: !7, size: 4096, elements: !8)
+!7 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed)
+!8 = !{!9}
+!9 = !DISubrange(count: 256)
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = distinct !DISubprogram(name: "overflow_iter_var", scope: !1, file: !1, line: 20, type: !12, scopeLine: 21, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!12 = !DISubroutineType(types: !13)
+!13 = !{null, !14, !14}
+!14 = !DIBasicType(name: "unsigned int", size: 16, encoding: DW_ATE_unsigned)
+!16 = !DILocalVariable(name: "i", scope: !17, file: !1, line: 23, type: !14)
+!17 = distinct !DILexicalBlock(scope: !11, file: !1, line: 23, column: 3)
+!18 = !DILocation(line: 0, scope: !17)
+!20 = !DILocalVariable(name: "stop1", arg: 1, scope: !11, file: !1, line: 20, type: !14)
+!21 = !DILocation(line: 0, scope: !11)