<div dir="ltr">> <span style="font-size:13px">I had thought that the case that Danny had looked at had a constant GEP, and so this constant might alias with other global pointers. How is that handled now?</span><div>That issue had to do with that we assumed that for all arguments of a given Instruction, each argument was either an Argument, GlobalValue, or Inst in `for (auto& Bb : Inst.getBasicBlockList()) for (auto& Inst : Bb.getInstList())`. ConstantExprs didn't fit into this instruction, because they aren't reached by said nested loop.</div><div><br></div><div>With this fix, if we detect that there's a relevant ConstantExpr, we'll look into it as if it were a regular Instruction inside of Bb.getInstList(), which causes us to correctly detect the globals, etc.</div><div><br></div><div>(I included a test case specifically for this -- it's ugly, but we have ~3 nested GEPs with a global at the innermost GEP. It produces the appropriate output)</div><div><br></div><div>George</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 30, 2015 at 10:56 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">----- Original Message -----<br>
> From: "George Burgess IV" <<a href="mailto:george.burgess.iv@gmail.com">george.burgess.iv@gmail.com</a>><br>
> To: "Daniel Berlin" <<a href="mailto:dberlin@dberlin.org">dberlin@dberlin.org</a>><br>
</span><span class="">> Cc: "Chandler Carruth" <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>, "Jiangning Liu"<br>
> <<a href="mailto:Jiangning.Liu@arm.com">Jiangning.Liu@arm.com</a>>, "LLVM Developers Mailing List" <<a href="mailto:llvmdev@cs.uiuc.edu">llvmdev@cs.uiuc.edu</a>><br>
</span><span class="">> Sent: Friday, January 30, 2015 8:15:55 AM<br>
> Subject: Re: [LLVMdev] question about enabling cfl-aa and collecting a57 numbers<br>
><br>
><br>
><br>
</span><span class="">> I'm not exactly thrilled about the size of this diff -- I'll happily<br>
> break it up into more manageable bits later today, because some of<br>
> it is test fixes, another bit is a minor bug fix, etc.<br>
<br>
</span>Yes, please break it into independent parts.<br>
<span class=""><br>
><br>
><br>
> Important bit (WRT ConstantExpr): moved the loop body from<br>
> buildGraphFrom into a new function. The body has a few tweaks to<br>
> call constexprToEdges on all ConstantExprs that we encounter.<br>
> constexprToEdges, naturally, interprets a ConstantExpr (and all<br>
> nested ConstantExprs) and places the results into a<br>
> SmallVector<Edge>.<br>
><br>
><br>
> I'm assuming this method of handling ConstantExprs isn't 100% correct<br>
> because I was told that handling them correctly would be more<br>
> difficult than I think it is. I can't quite figure out why, so<br>
> examples of cases that break my code would be greatly appreciated.<br>
> :)<br>
<br>
</span>I had thought that the case that Danny had looked at had a constant GEP, and so this constant might alias with other global pointers. How is that handled now?<br>
<br>
Thanks again,<br>
Hal<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
><br>
> George<br>
><br>
><br>
> On Mon, Jan 26, 2015 at 2:43 PM, George Burgess IV <<br>
> <a href="mailto:george.burgess.iv@gmail.com">george.burgess.iv@gmail.com</a> > wrote:<br>
><br>
><br>
><br>
><br>
> Inline<br>
><br>
><br>
> George<br>
><br>
><br>
><br>
><br>
> On Jan 26, 2015, at 1:05 PM, Daniel Berlin < <a href="mailto:dberlin@dberlin.org">dberlin@dberlin.org</a> ><br>
> wrote:<br>
><br>
><br>
> George, given that, can you just build constexpr handling (it's not<br>
> as easy as you think) as a separate funciton and have it use it in<br>
> the right places?<br>
> Will do. :)<br>
><br>
><br>
><br>
><br>
><br>
> FWIW, my current list of CFLAA issues is:<br>
><br>
> 1. Unknown values (results from ptrtoint, incoming pointers, etc) are<br>
> not treated as unknown. These should be done through graph edge (so<br>
> that they can be one way, otherwise, you will unify everything :P)<br>
><br>
><br>
><br>
><br>
> 2. Constexpr handling<br>
><br>
><br>
><br>
><br>
> ^^^ These are correctness issues. I'm pretty sure there are a few<br>
> more but i haven't finished auditing<br>
> 3. In a number of places we treat non-pointers as memory-locations<br>
> and unify them with pointers. This introduces a lot of spurious<br>
> aliasing.<br>
> 4. More generally, we induce a lot of spurious aliasing through<br>
> things at different dereference levels. In these cases, one may to<br>
> the other, but, for example, if we have a foo***, and a foo* (and<br>
> neither pointers to unknown things or escapes), the only way for foo<br>
> *** to alias foo* is if there is a graph path with two dereferences<br>
> between them.<br>
> We seem to get this wrong sometimes. Agreed on all four. Though<br>
> naturally it should be fixed, I’d like to see how much of an issue<br>
> #4 ends up being when we properly deal with #3.<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> On Sun Jan 25 2015 at 6:44:07 PM Chandler Carruth <<br>
> <a href="mailto:chandlerc@google.com">chandlerc@google.com</a> > wrote:<br>
><br>
><br>
><br>
><br>
><br>
><br>
> On Sun, Jan 25, 2015 at 6:37 PM, George Burgess IV <<br>
> <a href="mailto:george.burgess.iv@gmail.com">george.burgess.iv@gmail.com</a> > wrote:<br>
><br>
><br>
> > Fixing that still gives a wrong result, i haven't started to track<br>
> > down what *else* is going on here.<br>
><br>
><br>
> Running with the attached diff + a modified buildGraphFrom to handle<br>
> the constexpr GEPs, we seem to flag everything in test2.ll<br>
> (conservatively) correctly.<br>
><br>
><br>
> Is `store` the only place we can expect to see these constexpr<br>
> analogs, or is just about anywhere fair game?<br>
><br>
><br>
> Any Value can be a ConstantExpr, so all operands to instructions are<br>
> fair game.<br>
><br>
><br>
<br>
</div></div><div class="HOEnZb"><div class="h5">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</div></div></blockquote></div><br></div>