Hi, Thanks for the pointers. My patch now calls the attribute "noduplicate", and updates CodeMetrics to have another field: bool notDuplicatable; Which semantically is "containsIndirectBr || containsNoDuplicateInst". I didn't repurpose containsIndirectBr because I felt what I'm looking for is sufficiently different (indirectbr inhibits inlining, whereas noduplicate does not, if there is one call site). I still need to ensure InlineCost is correct; patch will be incoming tomorrow morning. All uses use CodeMetrics, except for JumpThreading because it has an early-exit mechanism as Nadav has explained. Cheers, James On Mon, 2012-12-03 at 06:27 +0000, Nadav Rotem wrote:> > On Dec 2, 2012, at 10:11 PM, Chris Lattner <clattner at apple.com> wrote: > > > 3) Please change random parts of the compiler to use CodeMetrics, > > instead of scattering random checks for this attribute throughout > > the code. Anything duplicating code and not using CodeMetrics is > > just plain incorrect. > > > One problem that we may run into when using CodeMetrics is compile > time. In many cases we are looking for one particular trait and can > exit as soon as we find it without having to scan the entire basic > block or function. For example, the jump threading pass stops > scanning additional instructions as soon as it passes the cost > threshold.
On Dec 3, 2012, at 9:48 AM, James Molloy <James.Molloy at arm.com> wrote:> Hi, > > Thanks for the pointers. My patch now calls the attribute "noduplicate", > and updates CodeMetrics to have another field: > > bool notDuplicatable; > > Which semantically is "containsIndirectBr || containsNoDuplicateInst". I > didn't repurpose containsIndirectBr because I felt what I'm looking for > is sufficiently different (indirectbr inhibits inlining, whereas > noduplicate does not, if there is one call site).I'm pretty sure that it's fine to inline indirectbr if there is a single call site and the inlinee is to be deleted. -Chris
Hi all + llvm-commits, After the discussion below, please find attached my patch to add a new "noduplicate" function attribute. I've modified CodeMetrics and LoopInfo, which covers most cases, but JumpThreading and InlineCost don't use CodeMetrics yet, so they required changing manually. Cheers, James On Mon, 2012-12-03 at 23:46 +0000, Chris Lattner wrote:> On Dec 3, 2012, at 9:48 AM, James Molloy <James.Molloy at arm.com> wrote: > > > Hi, > > > > Thanks for the pointers. My patch now calls the attribute "noduplicate", > > and updates CodeMetrics to have another field: > > > > bool notDuplicatable; > > > > Which semantically is "containsIndirectBr || containsNoDuplicateInst". I > > didn't repurpose containsIndirectBr because I felt what I'm looking for > > is sufficiently different (indirectbr inhibits inlining, whereas > > noduplicate does not, if there is one call site). > > I'm pretty sure that it's fine to inline indirectbr if there is a single call site and the inlinee is to be deleted. > > -Chris > >-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -------------- next part -------------- A non-text attachment was scrubbed... Name: noduplicate.diff Type: text/x-patch Size: 22069 bytes Desc: noduplicate.diff URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121204/932f832d/attachment.bin>