<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br><div><div>On Dec 20, 2012, at 8:02 AM, Manman Ren wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="content-type" content="text/html; charset=utf-8"><div dir="auto"><div>Ping</div><div><br>On Dec 18, 2012, at 4:13 PM, Manman Ren <<a href="mailto:mren@apple.com">mren@apple.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div><br></div><div>Stack Alignment: throw error if we can't satisfy the minimal alignment</div><div>requirement when creating stack objects in MachineFrameInfo.</div><div><br></div><div>Add CreateStackObjectWithMinAlign to throw error when the minimal alignment</div><div>can't be achieved and to clamp the alignment when the preferred alignment</div><div>can't be achieved. Same is true for CreateVariableSizedObject.</div><div>Will not emit error in CreateSpillStackObject or CreateStackObject.</div><div><br></div><div>As long as callers of CreateStackObject do not assume the object will be</div><div>aligned at the requested alignment, we should not have miscompile since</div><div>later optimizations looking at the object's alignment will have the correct</div><div>information.</div><div><br></div><div><br></div><div></div>
</div></blockquote><blockquote type="cite"><div><emit_error.patch></div></blockquote><blockquote type="cite"><div><div></div><div><br></div><div>Thanks,</div><div>Manman</div><div><br></div><div><div>On Dec 18, 2012, at 12:47 PM, Bob Wilson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 18, 2012, at 11:27 AM, Manman Ren <<a href="mailto:mren@apple.com">mren@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><br></div>There are some discussions in thread<div>Re: [llvm-commits] [llvm] r169197 - in /llvm/trunk: include/llvm/CodeGen/MachineFrameInfo.h include/llvm/Target/TargetFrameLowering.h lib/CodeGen/MachineFunction.cpp test/CodeGen/ARM/alloc-no-stack-realign.ll</div></div></blockquote><div><br></div>As you can tell, I'm way behind on patch reviews… Sorry that I missed the other discussion.</div><div><br></div><div>To Chris's comment about LLVMContext::emitError: I'm embarrassed to admit that I'd forgotten that was available for general error messages. I knew it was there for inline assembly, but didn't make the connection when I saw InstCombine writing a warning directly to errs(). I'll fix that and check for other places that may also need to be updated.</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><br></div><div><br></div><div><div><blockquote type="cite" style="font-size: 12px; margin: 0px;"><blockquote type="cite" style="margin: 0px; border-left-color: rgb(0, 99, 18); color: rgb(0, 99, 18);">When creating stack object, we use Align<br>Align = std::max((unsigned)TLI.getDataLayout()->getPrefTypeAlignment(Ty),<br> AI->getAlignment());<br>The preferred alignment may be too big for the target, and it makes sense to give a warning instead of a fatal error.<br></blockquote><br>If we can't realign the stack, we should print an error if<br>AI->getAlignment() is too large and simply ignore the return of<br>getPrefTypeAlignment if it's too large.<br></blockquote><br style="font-size: 12px;"><span style="font-size: 12px;">This means going through the call sites of these functions CreateStackObject … and checking the alignment there instead of</span><br style="font-size: 12px;"><span style="font-size: 12px;">wrapping it up in these functions.</span><br style="font-size: 12px;"><br style="font-size: 12px;"><span style="font-size: 12px;">Another option is to pass in an extra argument for the minimal alignment that must be satisfied.</span></div><div><span style="font-size: 12px;"><br></span></div><div><span style="font-size: 12px;">//////////////////////////////////////////////</span></div><div>--> I will pass an extra argument for the minimal alignment and will use emitError if the minimal alignment can't be satisfied and</div><div>will ignore the case where getPrefTypeAlignment is too large.</div><div>--> If that does not sound right, please let me know,</div><div><br></div><div>Thanks,</div><div>Manman</div><div><span style="font-size: 12px;"><br></span></div><div><div>On Dec 18, 2012, at 11:15 AM, Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br>On Dec 18, 2012, at 11:06 AM, Bob Wilson <<a href="mailto:bob.wilson@apple.com">bob.wilson@apple.com</a>> wrote:<br><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br>Hi Manman,<br><br>Could you move the functions from MachineFrameInfo.h first and then add the clamping code? Also, the debug output you emit should be within 'DEBUG()' macro. Like this:<br></blockquote>Hi Bill,<br>Sure, I will submit it as two patches and guard with DEBUG.<br><blockquote type="cite"><br><span class="Apple-tab-span" style="white-space:pre"> </span>DEBUG(dbgs() << "Warning: requested alignment " << Align<br><span class="Apple-tab-span" style="white-space:pre"> </span> << " exceeds the stack alignment " << StackAlign<br><span class="Apple-tab-span" style="white-space:pre"> </span> << " when stack realignment is off\n");<br><br>Or better yet make it an 'assert' if you don't expect this situation to ever occur:<br></blockquote>This can actually happen if the user specified a large alignment or the data type has a large alignment.<br></blockquote><br>In that case, using DEBUG is absolutely the wrong thing. The warning won't be produced in a compiler built without debugging. Is there any way you could detect this in the front-end and produce a proper diagnostic? (I haven't thought about that and haven't looked at the code, so forgive me if that's a stupid idea.) Otherwise, you just have to print out the warning to "errs()". In the long term, we need to figure out how to let the optimizer and code-gen produce real diagnostics (tracked at Apple in <<a href="rdar://problem/12867368">rdar://problem/12867368</a>>).<br></blockquote><br>Backend errors like this should be reported with LLVMContext::emitError.</blockquote></div><div><blockquote type="cite"><br>-Chris</blockquote></div><br></div></div></blockquote></div><br></div></blockquote></div><br></div></blockquote><blockquote type="cite"><div><span>_______________________________________________</span><br><span>llvm-commits mailing list</span><br><span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a></span><br><span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span><br></div></blockquote></div></blockquote></div><br></div></body></html>