<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">r228901 (bugfix+test) — the TODO was denoting that code under it is experimental; thanks for catching that. :)<div class=""><br class=""></div><div class="">(I need to start using branches more)<br class=""><div class=""><br class=""></div><div class="">George</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 11, 2015, at 5:20 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">----- Original Message -----</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">From: "George Burgess IV" <<a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a>><br class="">To: "Daniel Berlin" <<a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a>><br class="">Cc: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>>, "Chandler Carruth" <<a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a>>, "Jiangning Liu"<br class=""><<a href="mailto:Jiangning.Liu@arm.com" class="">Jiangning.Liu@arm.com</a>>, "Yogesh Chavan" <<a href="mailto:cs13m1012@iith.ac.in" class="">cs13m1012@iith.ac.in</a>>, "<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>for LLVM"<br class=""><<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a>><br class="">Sent: Wednesday, February 11, 2015 4:07:49 PM<br class="">Subject: Re: [LLVMdev] question about enabling cfl-aa and collecting a57 numbers<br class=""><br class=""><br class="">@Hal - Attached fix+test. Thanks for catching that<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">The bug fix and test case LGTM. Please commit those.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Regarding the new function in the patch:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+static bool canSkipAddingToSets(Value *V) {</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+ if (isa<Constant>(V) && !isa<GlobalValue>(V) && !isa<ConstantExpr>(V))</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+ return true;</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+ // TODO:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+ auto *Ty = V->getType();</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+ return Ty->isIntOrIntVectorTy();</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">+}</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Please don't commit that without filling in some text after the TODO :-)</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">-Hal</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">@Danny - Thanks<br class=""><br class=""><br class="">@Any - Assuming that we don't need to go back and revisit the patches<br class="">I submitted, the next thing on our to-do list seems to be removing<br class="">primitives as much as possible from set construction. Using CFLAA<br class="">with the supplied diffs, this can be as easy as modifying<br class="">`canSkipAddingToSets`.<br class=""><br class=""><br class="">I say "can be" because when we try to skip the adding of integers, a<br class="">fair number of the CodeGen tests fail [1]. These failures will only<br class="">occur if LLVM is bootstrapped by a compiler using solely CFLAA . So<br class="">I'll try to seek out a few cases where we're not giving the correct<br class="">answer and figure out why. :)<br class=""><br class=""><br class="">George<br class=""><br class=""><br class="">[1] - Failing tests:<br class=""><br class="">LLVM :: CodeGen/ARM/vector-spilling.ll<br class="">LLVM :: CodeGen/R600/scalar_to_vector.ll<br class="">LLVM :: CodeGen/R600/si-vector-hang.ll<br class="">LLVM :: CodeGen/Thumb2/2009-12-01-LoopIVUsers.ll<br class="">LLVM :: CodeGen/X86/avx-basic.ll<br class="">LLVM :: CodeGen/X86/avx-shuffle.ll<br class="">LLVM :: CodeGen/X86/avx-splat.ll<br class="">LLVM :: CodeGen/X86/avx-trunc.ll<br class="">LLVM :: CodeGen/X86/avx-unpack.ll<br class="">LLVM :: CodeGen/X86/avx-vpermil.ll<br class="">LLVM :: CodeGen/X86/avx2-conversions.ll<br class="">LLVM :: CodeGen/X86/avx2-shuffle.ll<br class="">LLVM :: CodeGen/X86/pointer-vector.ll<br class="">LLVM :: CodeGen/X86/psubus.ll<br class="">LLVM :: CodeGen/X86/sse3-avx-addsub.ll<br class="">LLVM :: CodeGen/X86/vec_cast2.ll<br class="">LLVM :: CodeGen/X86/vec_shuffle-27.ll<br class=""><br class=""><br class=""><br class=""><br class="">On Tue, Feb 10, 2015 at 4:27 PM, Daniel Berlin < <a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a><br class=""><blockquote type="cite" class="">wrote:<br class=""></blockquote><br class=""><br class=""><br class="">Sorry for the delay, i'll review these in the next few days.<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Tue Feb 10 2015 at 10:27:29 AM Hal Finkel < <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> ><br class="">wrote:<br class=""><br class=""><br class="">[moving to llvm-commits for patch review]<br class=""><br class="">----- Original Message -----<br class=""><blockquote type="cite" class="">From: "George Burgess IV" < <a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a> ><br class="">To: "Hal J. Finkel" < <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> ><br class="">Cc: "Chandler Carruth" < <a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a> >, "Jiangning Liu" <<br class=""><a href="mailto:Jiangning.Liu@arm.com" class="">Jiangning.Liu@arm.com</a> >, "LLVM Developers Mailing<br class="">List" < <a href="mailto:llvmdev@cs.uiuc.edu" class="">llvmdev@cs.uiuc.edu</a> >, "Daniel Berlin" <<br class=""><a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a> >, "Yogesh Chavan" < <a href="mailto:cs13m1012@iith.ac.in" class="">cs13m1012@iith.ac.in</a> ><br class="">Sent: Monday, February 9, 2015 11:25:38 AM<br class="">Subject: Re: [LLVMdev] question about enabling cfl-aa and<br class="">collecting a57 numbers<br class=""><br class=""><br class=""><br class="">As promised, attached are two diffs:<br class=""><br class=""><br class="">cflaa-asm-bugfix.diff fixes that we crashed given InlineAsm [1],<br class="">and<br class="">has a minor style update (remove trailing whitespace from comments)<br class=""></blockquote><br class="">Can you please attach a test case?<br class=""><br class=""><blockquote type="cite" class="">cflaa-inttoptr-fix.diff fixes how we treat inttoptr/ptrtoint<br class="">structures. I ended up implementing this with StratifiedAttrs, and<br class="">will speak with Danny about his concerns with this approach<br class="">(hopefully) later this week. It seems to meet the goal of not<br class="">unifying everything, so I don’t see it being problematic. [2]<br class=""></blockquote><br class="">Okay. I'd prefer that Danny's thoughts on this end up recorded<br class="">somewhere, so either we can do this on-list, or someone should<br class="">provide a summary.<br class=""><br class="">Thanks again,<br class="">Hal<br class=""><br class=""><blockquote type="cite" class=""><br class=""><br class="">Patches should be applied in the order noted above.<br class=""><br class=""><br class="">[1] - The bug was, more generally, when we’re given two Value*s<br class="">that<br class="">weren’t in any way related to a single function (e.g. two globals,<br class="">a<br class="">global + InlineAsm, …), we’d crash. Early in implementing CFLAA, I<br class="">wanted crashes because those are easy to detect/trace, and I had<br class="">minimal knowledge of what was getting passed in. Now that we have<br class="">an<br class="">impl that’s hopefully mostly working, a debug print will suffice in<br class="">these cases.<br class=""><br class=""><br class="">[2] - This implies that given %A and %B, where %A = inttoptr %Arg<br class="">(or<br class="">%Arg = ptrtoint %A), CFLAA *may* report NoAlias iff %B’s set has no<br class="">StratifiedAttrs.<br class=""><br class=""><br class="">Thanks again to Yogesh for the bug report,<br class="">George<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Feb 5, 2015, at 4:46 PM, George Burgess IV <<br class=""><a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a> > wrote:<br class=""><br class=""><br class="">+Yogesh -- he found bugs (llvm_unreachable reached -- looking into<br class="">why now) and said that he'd be interested in helping. Yay!<br class=""><br class=""><br class="">Status update: Toy implementation of properly handling<br class="">inttoptr/ptrtoint conversions (see: the fix for #2 on Danny's list)<br class="">passes tests. I need to do a bit of clean up (and want to test it<br class="">more thoroughly), and will hopefully have that patch out + a fix<br class="">for<br class="">the bug noted above by the end of the weekend<br class=""><br class=""><br class=""><br class="">George<br class=""><br class=""><br class="">On Wed, Feb 4, 2015 at 12:43 AM, George Burgess IV <<br class=""><a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a> > wrote:<br class=""><br class=""><br class=""><br class="">Sounds good, I'll reword that comment. Also, the assert you<br class="">mentioned<br class="">turned out to be a bad assumption when combined with how I foresee<br class="">us handling inttoptr/ptrtoint in the future, so I'll just replace<br class="">it<br class="">with slightly more robust code. :)<br class=""><br class=""><br class="">Thanks for the feedback,<br class="">George<br class=""><br class=""><br class=""><br class=""><br class="">On Tue, Feb 3, 2015 at 11:30 PM, Hal Finkel < <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> ><br class="">wrote:<br class=""><br class=""><br class="">Hi George,<br class=""><br class="">+// Given an Instruction, this will add it to the graph, along with<br class="">any<br class=""><br class="">+// Instructions that are potentially only available from said<br class="">Instruction<br class=""><br class="">I think this comment is somewhat misleading. You can't really have<br class="">orphaned instructions: instructions that have been inserted into a<br class="">basic block must appear in its linked list of instructions that<br class="">you'll visit when you iterate over all of them. You can have<br class="">constantexprs, and I think that's what you're try to say.<br class=""><br class="">+ assert(Edge.From == Inst.get() &&<br class=""><br class="">+ "Expected ConstantExpr edge `From` to evaluate to the<br class="">ConstantExpr");<br class=""><br class="">Indentation is odd here.<br class=""><br class="">For algorithmic considerations, I think that Danny is certainly the<br class="">best person to review these.<br class=""><br class="">-Hal<br class=""><br class="">----- Original Message -----<br class=""><blockquote type="cite" class="">From: "George Burgess IV" < <a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a> ><br class="">To: "Hal J. Finkel" < <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> ><br class="">Cc: "Chandler Carruth" < <a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a> >, "Jiangning Liu"<br class=""><<br class=""><a href="mailto:Jiangning.Liu@arm.com" class="">Jiangning.Liu@arm.com</a> >, "LLVM Developers Mailing<br class="">List" < <a href="mailto:llvmdev@cs.uiuc.edu" class="">llvmdev@cs.uiuc.edu</a> >, "Daniel Berlin" <<br class=""><a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a> ><br class="">Sent: Friday, January 30, 2015 10:34:43 PM<br class="">Subject: Re: [LLVMdev] question about enabling cfl-aa and<br class="">collecting a57 numbers<br class=""><br class=""><br class=""></blockquote><br class=""><br class=""><blockquote type="cite" class="">So, I split it up into three patches:<br class=""><br class=""><br class="">- cflaa-danny-fixes.diff are (some of?) the fixes that Danny gave<br class="">us<br class="">earlier for tests + the minimal modifications you’d need to make<br class="">in<br class="">CFLAA to make them pass tests.<br class="">- cflaa-minor-bugfixes.diff consists primarily of a bug fix for<br class="">Argument handling — we’d always report NoAlias when one of the<br class="">given<br class="">variables was an entirely unused argument<br class="">(We never added the appropriate Argument StratifiedAttr)<br class="">- cflaa-constexpr-fix.diff - The fix for the constexpr behavior<br class="">we’ve<br class="">been seeing<br class=""><br class=""><br class="">Patches are meant to be applied in the order listed.<br class=""><br class=""><br class="">Also, I just wanted to thank everyone again for your help so far<br class="">—<br class="">it’s greatly appreciated. :)<br class=""><br class=""><br class="">George<br class=""><br class=""><br class="">On Jan 30, 2015, at 11:56 AM, Hal Finkel < <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> ><br class="">wrote:<br class=""><br class="">----- Original Message -----<br class=""><br class=""><br class="">From: "George Burgess IV" < <a href="mailto:george.burgess.iv@gmail.com" class="">george.burgess.iv@gmail.com</a> ><br class="">To: "Hal Finkel" < <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> ><br class="">Cc: "Chandler Carruth" < <a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a> >, "Jiangning Liu"<br class=""><<br class=""><a href="mailto:Jiangning.Liu@arm.com" class="">Jiangning.Liu@arm.com</a> >, "LLVM Developers Mailing<br class="">List" < <a href="mailto:llvmdev@cs.uiuc.edu" class="">llvmdev@cs.uiuc.edu</a> >, "Daniel Berlin" <<br class=""><a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a><br class=""><blockquote type="cite" class=""><br class=""></blockquote>Sent: Friday, January 30, 2015 10:29:07 AM<br class="">Subject: Re: [LLVMdev] question about enabling cfl-aa and<br class="">collecting<br class="">a57 numbers<br class=""><br class="">I had thought that the case that Danny had looked at had a<br class="">constant<br class="">GEP, and so this constant might alias with other global pointers.<br class="">How is that handled now?<br class="">That issue had to do with that we assumed that for all arguments<br class="">of<br class="">a<br class="">given Instruction, each argument was either an Argument,<br class="">GlobalValue, or Inst in `for (auto& Bb :<br class="">Inst.getBasicBlockList())<br class="">for (auto& Inst : Bb.getInstList())`. ConstantExprs didn't fit<br class="">into<br class="">this instruction, because they aren't reached by said nested<br class="">loop.<br class=""><br class=""><br class="">With this fix, if we detect that there's a relevant ConstantExpr,<br class="">we'll look into it as if it were a regular Instruction inside of<br class="">Bb.getInstList(), which causes us to correctly detect the<br class="">globals,<br class="">etc.<br class=""><br class="">Sounds good.<br class=""><br class="">Thanks!<br class=""><br class="">-Hal<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">(I included a test case specifically for this -- it's ugly, but<br class="">we<br class="">have ~3 nested GEPs with a global at the innermost GEP. It<br class="">produces<br class="">the appropriate output)<br class=""><br class=""><br class="">George<br class=""><br class=""><br class="">On Fri, Jan 30, 2015 at 10:56 AM, Hal Finkel < hfinkel@anl.gov ><br class="">wrote:<br class=""><br class=""><br class="">----- Original Message -----<br class=""><br class=""><br class="">From: "George Burgess IV" < george.burgess.iv@gmail.com ><br class="">To: "Daniel Berlin" < dberlin@dberlin.org ><br class="">Cc: "Chandler Carruth" < chandlerc@google.com >, "Hal Finkel" <<br class="">hfinkel@anl.gov >, "Jiangning Liu"<br class="">< Jiangning.Liu@arm.com >, "LLVM Developers Mailing List" <<br class="">llvmdev@cs.uiuc.edu ><br class="">Sent: Friday, January 30, 2015 8:15:55 AM<br class="">Subject: Re: [LLVMdev] question about enabling cfl-aa and<br class="">collecting a57 numbers<br class=""><br class=""><br class=""><br class="">I'm not exactly thrilled about the size of this diff -- I'll<br class="">happily<br class="">break it up into more manageable bits later today, because some<br class="">of<br class="">it is test fixes, another bit is a minor bug fix, etc.<br class=""><br class="">Yes, please break it into independent parts.<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">Important bit (WRT ConstantExpr): moved the loop body from<br class="">buildGraphFrom into a new function. The body has a few tweaks to<br class="">call constexprToEdges on all ConstantExprs that we encounter.<br class="">constexprToEdges, naturally, interprets a ConstantExpr (and all<br class="">nested ConstantExprs) and places the results into a<br class="">SmallVector<Edge>.<br class=""><br class=""><br class="">I'm assuming this method of handling ConstantExprs isn't 100%<br class="">correct<br class="">because I was told that handling them correctly would be more<br class="">difficult than I think it is. I can't quite figure out why, so<br class="">examples of cases that break my code would be greatly<br class="">appreciated.<br class="">:)<br class=""><br class="">I had thought that the case that Danny had looked at had a<br class="">constant<br class="">GEP, and so this constant might alias with other global pointers.<br class="">How is that handled now?<br class=""><br class="">Thanks again,<br class="">Hal<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">George<br class=""><br class=""><br class="">On Mon, Jan 26, 2015 at 2:43 PM, George Burgess IV <<br class="">george.burgess.iv@gmail.com > wrote:<br class=""><br class=""><br class=""><br class=""><br class="">Inline<br class=""><br class=""><br class="">George<br class=""><br class=""><br class=""><br class=""><br class="">On Jan 26, 2015, at 1:05 PM, Daniel Berlin < dberlin@dberlin.org<br class=""><blockquote type="cite" class=""><br class=""></blockquote>wrote:<br class=""><br class=""><br class="">George, given that, can you just build constexpr handling (it's<br class="">not<br class="">as easy as you think) as a separate funciton and have it use it<br class="">in<br class="">the right places?<br class="">Will do. :)<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">FWIW, my current list of CFLAA issues is:<br class=""><br class="">1. Unknown values (results from ptrtoint, incoming pointers, etc)<br class="">are<br class="">not treated as unknown. These should be done through graph edge<br class="">(so<br class="">that they can be one way, otherwise, you will unify everything<br class="">:P)<br class=""><br class=""><br class=""><br class=""><br class="">2. Constexpr handling<br class=""><br class=""><br class=""><br class=""><br class="">^^^ These are correctness issues. I'm pretty sure there are a few<br class="">more but i haven't finished auditing<br class="">3. In a number of places we treat non-pointers as<br class="">memory-locations<br class="">and unify them with pointers. This introduces a lot of spurious<br class="">aliasing.<br class="">4. More generally, we induce a lot of spurious aliasing through<br class="">things at different dereference levels. In these cases, one may<br class="">to<br class="">the other, but, for example, if we have a foo***, and a foo* (and<br class="">neither pointers to unknown things or escapes), the only way for<br class="">foo<br class="">*** to alias foo* is if there is a graph path with two<br class="">dereferences<br class="">between them.<br class="">We seem to get this wrong sometimes. Agreed on all four. Though<br class="">naturally it should be fixed, I’d like to see how much of an<br class="">issue<br class="">#4 ends up being when we properly deal with #3.<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Sun Jan 25 2015 at 6:44:07 PM Chandler Carruth <<br class="">chandlerc@google.com > wrote:<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Sun, Jan 25, 2015 at 6:37 PM, George Burgess IV <<br class="">george.burgess.iv@gmail.com > wrote:<br class=""><br class=""><br class=""><br class=""><br class="">Fixing that still gives a wrong result, i haven't started to<br class="">track<br class="">down what *else* is going on here.<br class=""><br class=""><br class="">Running with the attached diff + a modified buildGraphFrom to<br class="">handle<br class="">the constexpr GEPs, we seem to flag everything in test2.ll<br class="">(conservatively) correctly.<br class=""><br class=""><br class="">Is `store` the only place we can expect to see these constexpr<br class="">analogs, or is just about anywhere fair game?<br class=""><br class=""><br class="">Any Value can be a ConstantExpr, so all operands to instructions<br class="">are<br class="">fair game.<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">--<br class="">Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<br class=""><br class=""><br class=""><br class="">--<br class="">Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<br class=""><br class=""></blockquote><br class="">--<br class="">Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<br class=""><br class=""><br class=""><br class=""><br class=""></blockquote><br class="">--<br class="">Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<br class=""><br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hal Finkel</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Assistant Computational Scientist</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Leadership Computing Facility</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Argonne National Laboratory</span></div></blockquote></div><br class=""></div></div></body></html>