<html dir="ltr">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style id="owaParaStyle" type="text/css">P {margin-top:0;margin-bottom:0;}</style>
</head>
<body ocsi="0" fpstyle="1">
<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;">> > Great! I think it's best to abandon this patch and then I'll post a patch that just does the<br>
> > encapsulation step. After that, I'll post another version of this patch starting from that point. Does that sound good?<br>
> Do note that what you're talking about as "the TargetMachine is passed down" only happens for<br>
> very few objects underneath TargetMachine and that generally things are constructed with the bare<br>
> minimum of information.<br>
<br>
I'm finding that some of these use more information than they first appear to but I agree that we should generally be passing information down rather than the whole object.<br>
<br>
> > > ... is that MCTargetMachine holds an owning pointer to MCCodeGenInfo, MCAsmInfo ...<br>
> > I've realised I made a mistake here. The pointers ought to be owned by the caller of MCTargetMachine::create*(). This is consistent with TargetMachine.<br>
> I don't agree with this part. I think the ownership hierarchy should be similar to TargetMachine in the way that TargetMachine owns TargetSubtargetInfo and it owns the things that depend upon it.<br>
<br>
Sorry, I've been a bit unclear here. I mean that the create*() functions themselves give ownership to their caller. TargetMachine holds some ownership resulting from LLVMTargetMachine's calls to TargetMachine's create*() functions.<br>
<br>
By the way, some of those members (e.g. MRI) look like they should be unique_ptr's. TargetMachine is manually deleting them in its destructor but nothing seems to set them aside from LLVMTargetMachine's calls to create*() in LLVMTargetMachine::initAsmInfo().
 Do you agree?<br>
<div> </div>
<div style="font-family: Times New Roman; color: #000000; font-size: 16px">
<hr tabindex="-1">
<div style="direction: ltr;" id="divRpF60409"><font face="Tahoma" color="#000000" size="2"><b>From:</b> Eric Christopher [echristo@gmail.com]<br>
<b>Sent:</b> 17 October 2015 12:17<br>
<b>To:</b> reviews+D13193+public+f5e72ebae342dcc9@reviews.llvm.org; Daniel Sanders; grosbach@apple.com<br>
<b>Cc:</b> hfinkel@anl.gov; jholewinski@nvidia.com; jfb@chromium.org; Matthew.Arsenault@amd.com; dschuff@google.com; jyknight@google.com; llvm-commits@lists.llvm.org; renato.golin@linaro.org<br>
<b>Subject:</b> Re: [PATCH] D13193: Add an MCTargetMachine and use it to configure MCAsmInfo<br>
</font><br>
</div>
<div></div>
<div>
<div dir="ltr"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Fri, Oct 16, 2015 at 7:23 AM Daniel Sanders <<a href="mailto:daniel.sanders@imgtec.com" target="_blank">daniel.sanders@imgtec.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex">
dsanders added a comment.<br>
<br>
Great! I think it's best to abandon this patch and then I'll post a patch that just does the encapsulation step. After that, I'll post another version of this patch starting from that point. Does that sound good?<br>
<br>
</blockquote>
<div><br>
</div>
<div>Do note that what you're talking about as "the TargetMachine is passed down" only happens for very few objects underneath TargetMachine and that generally things are constructed with the bare minimum of information.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex">
> ... is that MCTargetMachine holds an owning pointer to MCCodeGenInfo, MCAsmInfo ...<br>
<br>
<br>
I've realised I made a mistake here. The pointers ought to be owned by the caller of MCTargetMachine::create*(). This is consistent with TargetMachine.<br>
<br>
</blockquote>
<div><br>
</div>
<div>I don't agree with this part. I think the ownership hierarchy should be similar to TargetMachine in the way that TargetMachine owns TargetSubtargetInfo and it owns the things that depend upon it.</div>
<div><br>
</div>
<div>-eric</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex">
<br>
<a href="http://reviews.llvm.org/D13193" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13193</a><br>
<br>
<br>
<br>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</body>
</html>