<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 17, 2015 at 10:27 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com" target="_blank">clattner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Feb 17, 2015, at 1:58 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br>
> I'm not actually worried about this change though Chris, at least w.r.t. optimizer changes being necessary. There are a few reasons:<br>
><br>
> 1) The old ScalarRepl pass cared a *lot* about pointer type, but the new SROA doesn't care at all, so the biggest offender is essentially handled.<br>
<br>
</span>Why do you think that SRoA is the biggest “offender”?  This will pretty fundamentally changing the shape of the IR (in a good way) by presumably eliminating a ton of bitcasts etc.  This has the potential to provoke instcombine regressions, tickle things like globalopt and load/store elimination, etc.  I don’t think that any of these will be particularly difficult to fix, but I imagine that there will be a long tail of minor things.<br>
<span class=""><br>
> 2) We've recently changed our pointer canonicalization rules several times and in different ways. Each of those changes helped shake out bugs where the optimizer was relying on the pointer type for something. The number of things found has dropped dramatically with each change, so I don't think there is a huge pile of problems left hiding somewhere.<br>
<br>
</span>This is more reassuring for me.<br>
<span class=""><br>
> 3) Almost all of the problems we found with the changes to canonicalization were actually cases where *casts* impeded optimizations, not the different pointer type. This change will be a strict reduction in the need for casts, and thus I expect it to actually be safer than the other changes. All evidence is that most of the remaining reliance on these kinds of things are actually relying on an absence of casts. With this change, the casts will all go away.<br>
<br>
</span>Yes, I like this change for a number of reasons: reduction of casts, simplified type resolution stuff in libIR, etc.<br>
<span class=""><br>
> So, I'm not as worried about having a very drawn out period of fixing the optimizer. I think we'll probably uncover a few minor things that we have to fix immediately, and then when we make the change some small number of benchmarks will regress (likely on some small number of platforms). We'll have to track those down, no doubt, but I'm not worried about it preventing progress for a long time.<br>
<br>
</span>So long as the regressions are tracked down and fixed before the mega-patch is landed, I’m ok with making this change.  I just think that finding any ways to make it more incremental and stage it will be very well rewarded.  It will be impossible to review the resultant patch otherwise.<br></blockquote><div><br>It should come out somewhat incremental, I think. Here's how it's shaping up/I see it going:<br><br>1) add explicit types where required in IR instructions - gep, load, byval, anything else I can find... <br>  a) Provide the mechanism to specify it (in textual IR, bitcode, and LLVM IR APIs)<br>  b) Update callers to Clang, LLVM, and Polly, to pass that information (initially just asserting that it's the same information as was provided by the typed-pointer operands)<br>  c) /rely/ on that information in LLVM - stop using the pointee types<br><br>After that it might be monolithic - though I'll be trying to do it incrementally for my own sanity. <br><br><I was thinking I might remove ptr-to-ptr bitcasts here, before introducing the ptr type - since at this point they'll be pointless (har har) already, since the pointer-using instructions will no longer need the type from them anyway><br><br>2) introduce opaque pointer type (initially unused)<br>3) Repeat:<br>  a) choose an instruction or other source of pointer type<br>  b) update type to opaque pointer<br>  c) find & fix frontend bugs where it was relying on pointer type in the IR<br>  d) commit LLVM change<br><br>It could be monolithic there (do all the ptrs in one go), though I don't think it needs to be.<br><br>4) Remove non-opaque pointer types... - maybe. Might need to leave them in to make the back-compat bitcode reading easy, but I'm not sure.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-Chris</font></span></blockquote></div><br></div></div>