<div dir="ltr"><div style>Hi Paul,</div><div style><br></div><div style>Few comments:</div><div style><br></div><div>+; RUN: llc %s -o %t -filetype=obj -O0</div><div>+; RUN: llvm-dwarfdump %t | FileCheck %s -check-prefix=CHECK1</div>
<div>+; RUN: llvm-dwarfdump %t | FileCheck %s -check-prefix=CHECK2</div><div>+; PR14471</div><div><br></div><div>Can we get a comment on the two checks? I see why (or at least think I do) you've got them, but it's not necessarily obvious. I think it's because it makes it easier for you to check the things you're looking for.<br>
</div><div><br></div><div>+</div><div>+; ModuleID = 'debug-info-static-member.cpp'</div><div>+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"</div>
<div>+target triple = "x86_64-unknown-linux-gnu"</div><div><br></div><div style>Go ahead and use an -mtriple flag on the llc command and delete these lines. If there's no architecture specific need you can try to move it up to the next higher directory.</div>
<div style><br></div><div style><div>+  /// a typedef, a pointer or reference, etc.; or, a data member of a</div><div>+  /// class/struct/union.</div><div><br></div><div style>The ';' is awkward.</div></div><div style>
<br></div><div style><div>+/// getOrCreateContextDIE - Get context owner's DIE.</div><div>+DIE *CompileUnit::getOrCreateContextDIE(DIDescriptor Context)</div><div>+{</div><div><br></div><div style>Formatting.</div></div>
<div style><br></div><div style><div>+/// createStaticMemberType - Create debugging information entry for a</div><div>+/// C++ static data member.</div><div>+DIType DIBuilder::createStaticMemberType(DIDescriptor Scope, StringRef Name,</div>
<div>+                                         DIFile File, unsigned LineNumber,</div><div>+                                         DIType Ty, unsigned Flags,</div><div>+                                         llvm::Value *Val) {</div>
<div>+  assert(Flags & DIDescriptor::FlagStaticMember);</div><div>+  // TAG_member is encoded in DIDerivedType format.</div><div><br></div><div style>If you're going to create a new function for creating static member types why not just or the FlagStaticMember into the existing flags that are passed down rather than making sure it already exists?</div>
<div style><br></div><div style>Looks good otherwise.</div><div style><br></div><div style>-eric</div></div><div style><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jan 8, 2013 at 5:16 PM, Robinson, Paul <span dir="ltr"><<a href="mailto:Paul.Robinson@am.sony.com" target="_blank">Paul.Robinson@am.sony.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">New and improved patch for PR14471.<br>
<br>
- No more mock tag for static members; they use DW_TAG_member.<br>
- No more DIStaticDataMember; they use DIDerivedType.<br>
  This helps the scope of things somewhat, but I still have a number of<br>
  methods specific to static members; they are just different enough.<br>
- I had to revisit createGlobalVariableDIE(), because after staring at it,<br>
  I couldn't remember what it was doing or why.  It turns out it was not<br>
  completely correct, and now there's extra commentary to guide the unwary.<br>
- I figured out that the "isVariable" case under structures in<br>
  constructTypeDIE should be irrelevant now, so I took it out.<br>
- The test is beefed up.<br>
<br>
--paulr</blockquote></div><br></div>