Merge DIAssignID attachments on stores that are merged and sunk out of loops. The store may be sunk into multiple exit blocks, and in this case all the copies of the store get the same DIAssignID.
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
LGTM; you might want to delete tbaa but it's not too important. What's the behaviour if there are multiple exit paths from the loop that store different values, like
if (foo) {
var = 1; break;
} else if (bar) {
var = 2; break;
} ...Presumably there'll be an exit block with a PHI, then a store -- and all the dbg.assigns for 'var' will point at that final store, but will have different value operands? (Just trying to work out what this would look like in my mind.
| llvm/lib/Transforms/Scalar/LICM.cpp | ||
|---|---|---|
| 1890 | I feel it'd be helpful to have a comment here saying "NewSI will have it's DIAssignID set here if there are any labelled stores in Uses", or something, so that the reader can work out where the non-null DIAssignID comes from. | |
| llvm/test/DebugInfo/Generic/assignment-tracking/licm/merge.ll | ||
| 17 | CHECK-LABEL? (and the next block check) | |
| llvm/test/DebugInfo/Generic/assignment-tracking/licm/multi-exit.ll | ||
| 23–24 | This seems slightly un-natural as there aren't any loop exits? | |
| llvm/test/DebugInfo/Generic/assignment-tracking/licm/multi-exit.ll | ||
|---|---|---|
| 23–24 | IIRC (it's been a while since I created this test) this was reduced from a real-world reproducer and the IR hasn't been modified. It looks like this CFG has become two loops and we're concerned with the inner one, which appears to look like this: if.cond - header, exiting Here's the CFG. entry
|
v
*---->for.cond.outer
| |
| v
| +->for.cond-----+
| | | |
| | v |
| +--if.then |
| | |
| v v
| if.end if.end3.loopexit
| | |
| v |
+-----if.end3 <----+The store in for.cond is sunk into the inner-loop exits if.end and if.end3.loopexit. | |
Comment Actions
+ Address review comments
+ Add CFG diagram to test multi-exit.ll
you might want to delete tbaa but it's not too important.
Done.
Yep that's right, here's a reproducer with IR:
int a, b;
void esc(int*);
int fun(int if_cond, int loop_cond) {
int local = 0;
while (--loop_cond) {
if (if_cond) {
local = a;
} else {
local = b;
esc(&a);
}
}
esc(&local);
return local;
}Before LICM
... while.body: ; preds = %while.cond %tobool1.not = icmp eq i32 %if_cond, 0, !dbg !34 br i1 %tobool1.not, label %if.else, label %if.then, !dbg !37 if.then: ; preds = %while.body %1 = load i32, i32* @a, align 4, !dbg !38, !tbaa !26 store i32 %1, i32* %local, align 4, !dbg !40, !tbaa !26, !DIAssignID !41 call void @llvm.dbg.assign(metadata i32 %1, metadata !19, metadata !DIExpression(), metadata !41, metadata i32* %local, metadata !DIExpression()), !dbg !21 br label %if.end, !dbg !42 if.else: ; preds = %while.body %2 = load i32, i32* @b, align 4, !dbg !43, !tbaa !26 store i32 %2, i32* %local, align 4, !dbg !45, !tbaa !26, !DIAssignID !46 call void @llvm.dbg.assign(metadata i32 %2, metadata !19, metadata !DIExpression(), metadata !46, metadata i32* %local, metadata !DIExpression()), !dbg !21 tail call void @_Z3escPi(i32* noundef nonnull @a), !dbg !47 br label %if.end if.end: ; preds = %if.else, %if.then br label %while.cond, !dbg !31, !llvm.loop !48 ; Exit blocks while.end: ; preds = %while.cond call void @_Z3escPi(i32* noundef nonnull %local), !dbg !51 ...
After LICM
... while.body: ; preds = %while.cond %tobool1.not = icmp eq i32 %if_cond, 0, !dbg !34 br i1 %tobool1.not, label %if.else, label %if.then, !dbg !37 if.then: ; preds = %while.body %2 = load i32, i32* @a, align 4, !dbg !38, !tbaa !26 call void @llvm.dbg.assign(metadata i32 %2, metadata !19, metadata !DIExpression(), metadata !40, metadata i32* %local, metadata !DIExpression()), !dbg !21 br label %if.end, !dbg !41 if.else: ; preds = %while.body %3 = load i32, i32* @b, align 4, !dbg !42, !tbaa !26 call void @llvm.dbg.assign(metadata i32 %3, metadata !19, metadata !DIExpression(), metadata !40, metadata i32* %local, metadata !DIExpression()), !dbg !21 tail call void @_Z3escPi(i32* noundef nonnull @a), !dbg !44 br label %if.end if.end: ; preds = %if.else, %if.then %4 = phi i32 [ %3, %if.else ], [ %2, %if.then ] br label %while.cond, !dbg !31, !llvm.loop !45 ; Exit blocks while.end: ; preds = %while.cond %.lcssa = phi i32 [ %1, %while.cond ] store i32 %.lcssa, i32* %local, align 1, !dbg !48, !tbaa !26, !DIAssignID !40 call void @_Z3escPi(i32* noundef nonnull %local), !dbg !49 ...
I feel it'd be helpful to have a comment here saying "NewSI will have it's DIAssignID set here if there are any labelled stores in Uses", or something, so that the reader can work out where the non-null DIAssignID comes from.