[llvm] r245394 - [BasicAA] Revert r221876 because it can produce incorrect aliasing

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Quentin Colombet" <qcolombet at apple.com>
> To: "Nicholas White" <n.j.white at gmail.com>
> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>, "Michael Zolotukhin" <mzolotukhin at apple.com>, "Hal Finkel"
> <hfinkel at anl.gov>
> Sent: Wednesday, August 26, 2015 11:29:35 AM
> Subject: Re: [llvm] r245394 - [BasicAA] Revert r221876 because it can produce incorrect aliasing
> 
> 
> Hi Nick,
> 
> How do we move forward here?
> 
> 
> My impression is that GEP analysis are not robust enough.
> Would it make sense to just disable them until we fix them?
> Is that doable?

I could be wrong, but it seems like the problems are specifically related to the GEP-based analysis that look through zext/sext operands. In which case, the relevant logic should be easy to disable.

Let's figure out whether Nick's latest patch was actually buggy, or if it just exposed some other unrelated problem. We might actually be at the end of the tunnel here.

 -Hal

> 
> Cheers,
> -Quentin
> 
> 
> 
> 
> 
> 
> 
> On Aug 26, 2015, at 6:06 AM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> ----- Original Message -----
> 
> 
> From: "Nicholas White" < n.j.white at gmail.com >
> To: "Michael Zolotukhin" < mzolotukhin at apple.com >
> Cc: "Quentin Colombet" < qcolombet at apple.com >,
> llvm-commits at lists.llvm.org , "Hal Finkel" < hfinkel at anl.gov >
> Sent: Wednesday, August 26, 2015 2:53:00 AM
> Subject: Re: [llvm] r245394 - [BasicAA] Revert r221876 because it can
> produce incorrect aliasing
> 
> Sure -
> https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11847&d=BQICaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=xmbDP42uBBBlalGksUB_L-z5bVwg-Vr8dgKN8EA5xiA&s=vy0GPUDE70AmMPXLM9lXWFsPkbltPzcjoFIuZphyf9M&e=
> is the most recent version of
> my
> Sext / Zext handling fixes, and has all the edge cases I've found to
> date. It was reverted above due to a bug in it, which I've tracked
> down to the below. %arrayidx3 and %ra are MayAlias in 3.6.1 but
> NoAlias in my most recent patch.
> 
> define void @benchmark_heapsort(i32 %n, double* nocapture %ra) {
> %shr = ashr i32 %n, 1
> %add = add nsw i32 %shr, 1
> %arrayidx3 = getelementptr inbounds double, double* %ra, i64 1
> ret void
> }
> 
> Also, is there an easy way of measuring code coverage of an llvm-lit
> run? Given the number of reversions of this patch I think there are
> large coverage gaps, as each iteration has passed all the BasicAA
> unit
> tests :(
> 
> Overall coverage from the entire set of regression tests is not bad (
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_reports_coverage_lib_Analysis_BasicAliasAnalysis.cpp.gcov.html&d=BQICaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=xmbDP42uBBBlalGksUB_L-z5bVwg-Vr8dgKN8EA5xiA&s=dBRcZxF0G3rE15nyguByf7siqLid7b2E8R0wOgGTclU&e=
> ), but coverage itself is only a small part of the story. Some tests
> are imprecise, and it is really the various combinations of control
> flow paths that is the problem.
> 
> -Hal
> 
> 
> 
> 
> Thanks -
> 
> Nick
> 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory