<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Sep 18, 2009, at 8:55 AM, Victor Hernandez wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div><blockquote type="cite"><div><br>Index: lib/Analysis/ValueTracking.cpp<br>===================================================================<br>--- lib/Analysis/ValueTracking.cpp<span class="Apple-tab-span" style="white-space: pre; "> </span>(revision 81898)<br>+++ lib/Analysis/ValueTracking.cpp<span class="Apple-tab-span" style="white-space: pre; "> </span>(working copy)<br>@@ -20,6 +20,7 @@<br>#include "llvm/IntrinsicInst.h"<br>#include "llvm/LLVMContext.h"<br>#include "llvm/Operator.h"<br>+#include "llvm/Analysis/MallocHelper.h"<br>#include "llvm/Target/TargetData.h"<br>#include "llvm/Support/GetElementPtrTypeIterator.h"<br>#include "llvm/Support/MathExtras.h"<br>@@ -621,6 +622,24 @@<br> break;<br> }1<br> }<br>+ } else if (CallInst* CI = extractMallocCall(I)) {<br>+ unsigned Align = 0;<br>+ const Type* T = getMallocAllocatedType(CI);<br>+ if (TD && T) {<br>+ // Malloc returns maximally aligned memory.<br>+ Align = TD->getABITypeAlignment(T);<br>+ Align =<br>+ std::max(Align,<br>+ (unsigned)TD->getABITypeAlignment(<br>+ Type::getDoubleTy(V->getContext())));<br>+ Align =<br>+ std::max(Align,<br>+ (unsigned)TD->getABITypeAlignment(<br>+ Type::getInt64Ty(V->getContext())));<br>+ }<br>+ if (Align > 0)<br>+ KnownZero = Mask & APInt::getLowBitsSet(BitWidth,<br>+ CountTrailingZeros_32(Align));<br> }<br> break;<br> }<br><br>This is simply wrong; we can't make that guarantee for malloc.<br>Consider, for example, a malloc used as an SSE vector on Windows.<font class="Apple-style-span"><font class="Apple-style-span" color="#144FAE"><br></font></font></div></blockquote><div><br></div>I agree that this is wrong; yet the bug you describe currently exists in LLVM TOT and results in better codegen on some systems. RaiseAllocations creates a MallocInst with no specified alignment. KnownZero for MallocInst ends up getting the wrong alignment, just like malloc calls would with this code. </div><div><br></div><div>I am not sure what to do in this scenario. I agree that getting rid of this code is correct for the example you mention, but the code is currently resulting in aligned instructions being generated on MacOSX/x86, so it's tempting to keep it in.</div></span><br class="Apple-interchange-newline"></blockquote><br></div><div>After discussing this further, I am not going to persist the current malloc alignment bug. I'll won't commit this patch, and file a bug to extend TargetData to identify the target-specific malloc alignment.</div><div><br></div><div>Victor</div><div><br></div><br></body></html>