<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">LGTM!<div><br></div><div>This should work even for the combination of muddles Renato mentions, as we're re-initializing the value for each MachineFunction we process.</div><div><br></div><div>-Jim</div><div><br><div><div>On Dec 18, 2012, at 1:22 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hi,</div><div><br></div>Thanks for the reviews.<div><br></div><div>I'd like Jim's idea about caching the information in a local class variable.</div><div>I've updated the patch accordingly.</div><div><br></div><div>New patch attached.</div><div><br><div apple-content-edited="true">
<div style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">-Quentin</div><div style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br></div><div style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "></div></div></div></div><span><Thumb2SizeReduction-Oz.patch></span><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div apple-content-edited="true"><div style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "></div>
</div>
<br><div><div>On Dec 18, 2012, at 1:00 PM, Renato Golin <<a href="mailto:rengolin@systemcall.org">rengolin@systemcall.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On 18 December 2012 20:18, Jim Grosbach <<a href="mailto:grosbach@apple.com">grosbach@apple.com</a>> wrote:<br><blockquote type="cite">The command line options aren't really available here, and we'd have to check for a function-local override anyway, so I don't think that'll work out.<br></blockquote><br>True, I forgot about that.<br><br><br><blockquote type="cite">Is there concern here the potential compile time impact? Accessing the attribute list should be pretty quick (a couple of pointer indirections and a bit mask).<br></blockquote><br>Mainly, yes, but I agree it's not a big problem.<br><br><br><blockquote type="cite">If we want, we could cache the value locally in the size reduction pass as a local class ivar and initialize it at the start of runOnMachineFunction(). That would make the access a single pointer indirection off the 'this' pointer for the pass.<br></blockquote><br>If that pass would only run on a single module, that'd make sense. But<br>if you could run it (opt) on a collation of different modules,<br>possibly compiled with different flags, than I'd think you'd need the<br>full check.<br><br>I agree that the lookup is cheap, sorry for having over-complicated things. ;)<br><br>LGTM.<br><br>cheers,<br>--renato<br></blockquote></div><br></div></blockquote></div><br></div></body></html>