This is an archive of the discontinued LLVM Phabricator instance.

Differential D47720

[DebugInfo] Inline for without DebugLocation
ClosedPublic

Authored by gramanas on Jun 4 2018, 8:08 AM.

Details

Reviewers
vsk
Commits
rGdfe8fe503cbe: [DebugInfo] Inline for without DebugLocation
rC335255: [DebugInfo] Inline for without DebugLocation
rL335255: [DebugInfo] Inline for without DebugLocation
Summary

This test is a strip down version of a function inside the
amalgamated sqlite source. When converted to IR clang produces
a phi instruction without debug location.

It is a continuation of D47097 focusing on a different problem
of similar structure.

This was found while investigating instructions with missing DebugLoc
after running SROA on the amalgamated sqlite source, in an attempt to
try and preserve as much debug information as possible during the
early stage passes.

Diff Detail

Event Timeline

gramanas created this revision.Jun 4 2018, 8:08 AM
Herald added subscribers: cfe-commits, JDevlieghere, eraman, aprantl. · View Herald TranscriptJun 4 2018, 8:08 AM
aprantl added inline comments.Jun 4 2018, 8:54 AM
test/CodeGen/debug-info-inline-for.c
3

Please add a comment explaining what is being tested here.

gramanas updated this revision to Diff 149996.Jun 5 2018, 9:00 AM

Add comment explaining the test

Harbormaster completed remote builds in B18955: Diff 149996.Jun 5 2018, 9:00 AM
gramanas marked an inline comment as done.Jun 5 2018, 9:00 AM
aprantl added inline comments.Jun 5 2018, 10:03 AM
test/CodeGen/debug-info-inline-for.c
14

Shouldn't you also check *which* location is attached to it?

gramanas updated this revision to Diff 150036.Jun 5 2018, 1:47 PM
gramanas marked an inline comment as done.

Add artificial debug location to the phi instruction

test/CodeGen/debug-info-inline-for.c
14

I wasn't sure if this should be an artificial location or not so I left it like that for the time being. It's been updated to reflect the latest changes now.

vsk accepted this revision.Jun 5 2018, 1:59 PM

As is the case with D47097, this helps preserve more scope information after SROA. LGTM. Thanks!

This revision is now accepted and ready to land.Jun 5 2018, 1:59 PM
gramanas updated this revision to Diff 150135.Jun 6 2018, 7:50 AM

make code more readable

Harbormaster completed remote builds in B18992: Diff 150135.Jun 6 2018, 7:51 AM
gramanas added a comment.Jun 19 2018, 7:35 AM

ping! should I commit this?

probinson added a subscriber: probinson.Jun 20 2018, 9:38 AM
In D47720#1136585, @gramanas wrote:

ping! should I commit this?

@vsk accepted it, so yes go ahead.

Closed by commit rL335255: [DebugInfo] Inline for without DebugLocation (authored by gramanas). · Explain WhyJun 21 2018, 9:58 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: llvm-commits. · View Herald TranscriptJun 21 2018, 9:58 AM
dblaikie mentioned this in D91734: [FastISel] Flush local value map on every instruction.Dec 2 2020, 3:19 PM

Revision Contents

PathSize
lib/
CodeGen/
6 lines
test/
CodeGen/
13 lines

Diff 150135

lib/CodeGen/CGExprScalar.cpp

Show First 20 Lines • Show All 3,422 Lines • ▼ Show 20 LinesValue *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
{ {
// There is no need to emit line number for unconditional branch. // There is no need to emit line number for unconditional branch.
auto NL = ApplyDebugLocation::CreateEmpty(CGF); auto NL = ApplyDebugLocation::CreateEmpty(CGF);
CGF.EmitBlock(ContBlock); CGF.EmitBlock(ContBlock);
} }
// Insert an entry into the phi node for the edge with the value of RHSCond. // Insert an entry into the phi node for the edge with the value of RHSCond.
PN->addIncoming(RHSCond, RHSBlock); PN->addIncoming(RHSCond, RHSBlock);
// Artificial location to preserve the scope information
{
auto NL = ApplyDebugLocation::CreateArtificial(CGF);
PN->setDebugLoc(Builder.getCurrentDebugLocation());
}
// ZExt result to int. // ZExt result to int.
return Builder.CreateZExtOrBitCast(PN, ResTy, "land.ext"); return Builder.CreateZExtOrBitCast(PN, ResTy, "land.ext");
} }
Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) {
// Perform vector logical or on comparisons with zero vectors. // Perform vector logical or on comparisons with zero vectors.
if (E->getType()->isVectorType()) { if (E->getType()->isVectorType()) {
CGF.incrementProfileCounter(E); CGF.incrementProfileCounter(E);
▲ Show 20 Lines • Show All 627 Lines • Show Last 20 Lines

test/CodeGen/debug-info-inline-for.c

  • This file was added.
// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
// Check that clang emits Debug location in the phi instruction
aprantlUnsubmitted
Done
ReplyInline Actions

Please add a comment explaining what is being tested here.

aprantl: Please add a comment explaining what is being tested here.
int func(int n) {
int a;
for(a = 10; a>0 && n++; a--);
return n;
}
// CHECK: land.end:
// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]]
// CHECK: ![[DbgLoc]] = !DILocation(line: 0
aprantlUnsubmitted
Done
ReplyInline Actions

Shouldn't you also check *which* location is attached to it?

aprantl: Shouldn't you also check *which* location is attached to it?
gramanasAuthorUnsubmitted
Not Done
ReplyInline Actions

I wasn't sure if this should be an artificial location or not so I left it like that for the time being. It's been updated to reflect the latest changes now.

gramanas: I wasn't sure if this should be an artificial location or not so I left it like that for the…