<div dir="ltr">On 26 August 2013 01:40, Puyan Lotfi <span dir="ltr"><<a href="mailto:plotfi@apple.com" target="_blank">plotfi@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>All:</div><div><br></div><div>Attached is a patch to GlobalOpt that adds an optimization that takes user written constant initialization code for global variables and hoists the initialization values into the global initializer. This optimization is only done on locally linked globals of integral types (scalars, arrays, and structs) that are constant initialized (and zero initialized prior to transformation). </div>
<div><br></div><div>The idea is to transform the following code:</div><div><br></div><div><div><font face="Courier New">A = internal global i32 0, align 4</font></div><div><font face="Courier New">isInit = internal global i1 false</font></div>
<div><font face="Courier New"><br></font></div><div><font face="Courier New">define i32* foobar() {</font></div><div><font face="Courier New"> %.b = load i1* isInit</font></div><div><font face="Courier New"> %1 = zext i1 %.b to i8</font></div>
<div><font face="Courier New"> %2 = trunc i8 %1 to i1</font></div><div><font face="Courier New"> br i1 %2, BB4, label BB3</font></div><div><font face="Courier New">BB3:</font></div><div><font face="Courier New"> store i32 113, i32* A, align 4</font></div>
<div><font face="Courier New"> store i1 true, i1* isInit</font></div><div><font face="Courier New"> br label %4</font></div><div><font face="Courier New">BB4:</font></div><div><font face="Courier New"> ret i32* A</font></div>
<div><font face="Courier New">}</font></div><div><br></div><div>Into:</div><div><font face="Courier New"><br></font></div><div><font face="Courier New">A = internal global i32 113, align 4</font></div><div><font face="Courier New"><br>
</font></div><div><font face="Courier New">define i32* @_Z8initTestv() {</font></div><div><font face="Courier New"> ret i32* A</font></div><div><font face="Courier New">}</font></div><div><br></div></div><div>Could someone on the list review my changes, provide feedback, and if possible submit my changes?</div>
</div></blockquote><div><br></div><div>You'll need to provide testcases before we can submit it for you.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div></div><div>I also have some test cases I've written but I am still trying to figure out how to add them to llvm/test/Transforms/GlobalOpt (I don't see a lit.local.cfg in that directory as the docs specify). </div>
</div></blockquote><div><br></div><div><div>+/// variables and if the condtions are right proceeds to move the init</div></div><div><br></div><div>Typo, "condtions" --> "conditions"</div><div><br></div>
<div><div>+/// The following is an example of code we would like to optimize:</div><div>+/// S* init () {</div><div>+/// static bool isInit = false; static S s;</div><div>+/// if (!isInit) { s.x = 42; s.y = 21; isInit = true; }</div>
<div>+/// return &s;</div><div>+/// }</div></div><div><br></div><div>I'm curious, is this a pattern people actually write?</div><div><br></div><div><div>+static bool TryToHoistGlobalConstantInitializers(GlobalVariable *GV) {</div>
<div>+ bool isInitCandidate = true;</div><div>+ BasicBlock* defBB = NULL;</div></div><div><br></div><div>"BasicBlock* defBB" --> "BasicBlock *defBB".</div><div><br></div><div><div>+ std::vector<ReturnInst*> GVreturnInstUses;</div>
<div>+ std::vector<StoreInst*> GVstoreInstUses;</div><div>+ std::set<Function*> initFunction; // weed out dupes</div></div><div><br></div><div>Please see <a href="http://llvm.org/docs/ProgrammersManual.html#picking-the-right-data-structure-for-a-task">http://llvm.org/docs/ProgrammersManual.html#picking-the-right-data-structure-for-a-task</a> .</div>
<div><br></div><div><div>+ // Only consider globals that are interally linked, and constant initialized.</div></div><div><br></div><div>Typo, "interally" --> "internally".</div><div><br></div><div>
<div>+ // also return a referenece.</div></div><div><br></div><div>Typo, "referenece" --> "reference".</div><div><br></div><div><div>+ bool GVChanged;</div><div>+ do {</div><div>+ GVChanged = false;</div>
<div>+ for (Module::global_iterator GVI = M.global_begin(),</div><div>+ E = M.global_end(); GVI != E; ++GVI) {</div><div>+ if(TryToHoistGlobalConstantInitializers(GVI)) {</div><div>+ Changed |= GVChanged = true;</div>
<div>+ break;</div><div>+ }</div><div>+ }</div><div>+ } while (GVChanged);</div></div><div><br></div><div>No. Iterate until convergence is very bad style in the compiler because we can't prove that it has less than O(n^2) complexity (or alternatively there are a few places where it is O(n^2) complexity and we believe that's the best we can do).</div>
<div><br></div><div>Also, there are a few place where your patch seems unfinished:</div><div><br></div><div><div>+ if (!attemptGlobalInitHoist) return false;</div><div>+ //////////////////////////////////////////////////////////////////////////////</div>
</div><div><br></div><div>Is this a marker indicating where you wanted to split into separate functions?</div><div><br></div><div><div>+ if (defBB->size() != 1) {</div><div>+ /// Assert?</div><div>+ }</div>
</div>
<div><br></div><div>Decide this before sending out the patch. If you decide not to assert, you should probably include a testcase that shows why (ie., that would trip the assert, demonstrating why the assert is wrong).</div>
<div><br></div><div>Nick</div><div><br></div></div></div></div>