<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 4, 2019 at 12:18 PM Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Alex,<br>
<br>
Thanks for reporting this.<br>
Wei worked on the hoisting optimization.<br>
<br>
@Wei, could you work with Alex to see what is the problem.<br>
<br>
Cheers,<br>
-Quentin<br>
<br>
> On Nov 3, 2019, at 5:20 AM, via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> <br>
>       /// Optimizations after all the reg selections and spills are done.<br>
>       void InlineSpiller::postOptimization() { HSpiller.hoistAllSpills();<br>
> }<br>
> <br>
> Seems a problematic function to me, as hoistAllSpills() uses<br>
> TII.storeRegToStackSlot() to insert new spills.<br>
> <br>
> The problem is, TII.storeRegToStackSlot is allowed to create new virtual<br>
> registers, which can not be allocated a range as this whole thing is called<br>
> _after_ all reg selection is complete.<br>
> <br>
> If I'm right in this, I do not see how the in-tree target AMDGPU::SI has not<br>
> been affected, as it creates virtual registers in both load and store stack<br>
> operations in SIInstrInfo.cpp - which is where I confirmed to myself that it<br>
> was okay to do so. When compilation broke,<br>
> <a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130812/184331.htm" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130812/184331.htm</a><br>
> l further suggested that the intention is that you can... but I do not see<br>
> how a hoist can ever pass verification/compile correctly. Am I doing<br>
> something incorrectly here? Or are hoistable spills just that rare on GPU<br>
> code that it's never come up?<br></blockquote><div><br></div><div>Hi Alex, </div><div><br></div><div>I havn't noticed before that TII.storeRegToStackSlot is allowed to create new virtual registers. I am surprised to know it is allowed to do that. I am mostly working on x86 and TII.storeRegToStackSlot for x86 doesn't create new register. As you said, if TII.storeRegToStackSlot is allowed to create new virtual registers, that could be a problem. It is not exposed maybe because of some later pass cover the problem -- maybe RegScavenger? </div><div><br></div><div>Are you aware of any other architecture other than AMDGPU on which TII.storeRegToStackSlot creates new virtual registers? Could you explain in which case new virtual registers will be created? </div><div><br></div><div>Thanks,</div><div>Wei.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> As a side note, compiler option "-disable-spill-hoist" (DisableHoisting) is<br>
> referenced in exactly zero places, so if anyone in a similar situation finds<br>
> this post, maybe don't bother testing with that flag just yet. :)<br>
>       <br>
> <br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
</blockquote></div></div>