Wrong value for sext i1 true in BasicAliasAnalysis

[Hal Finkel]

Hi Yin,

Thanks! I agree that something here is clearly wrong :-)

So we're trying to evaluate the aliasing of:

(gdb) call LocA.Ptr->dump()
  %pos.i63 = getelementptr inbounds [3 x %struct.spec_fd_t]* @spec_fd, i32 0, i32 %3, i32 2

where:

  %3 = zext i1 true to i32

vs.

(gdb) call LocB.Ptr->dump()
i32* getelementptr inbounds ([3 x %struct.spec_fd_t]* @spec_fd, i32 0, i32 1, i32 2)

and these are identical, and should alias.

Now when we get near line 265 in BasicAliasAnalysis.cpp, we have:

(gdb) call V->dump()
  %3 = zext i1 true to i32

we set Extension = EK_ZeroExt, and call GetLinearExpression on (i1 true). This returns (i1 true) with Scale = (i32 0) and Offset = (i1 1). And then we get there:

266	    // We have to sign-extend even if Extension == EK_ZeroExt as we can't
267	    // decompose a sign extension (i.e. zext(x - 1) != zext(x) - zext(-1)).
268	    Offset = Offset.sext(OldWidth);

and we sign extend the offset from (i1 1) to (i32 -1) -- leading to an overall return of NoAlias.

Here's the issue: I don't think this has anything, in particular, to do with special casing i1 values. There seems to just be something fundamentally wrong with the logic here. For one thing, I can easily modify the test case you provided to do the wrong thing for a zext of an i8 value (attached).

I also don't understand, for example, why it is correct to truncate and re-extend the already-computed Offset and Scale from the caller. I've cc'd Nick White, who wrote the patch that touched this last.

 -Hal

----- Original Message -----
> From: "Yin Ma" <yinma at codeaurora.org>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Wednesday, December 10, 2014 12:54:42 PM
> Subject: RE: Wrong value for sext i1 true in BasicAliasAnalysis
> 
> Hi Hal,
> 
> I have created a simple test case for this case.
> opt -basicaa -gvn -S q.ll
> 
> q.bad.ll shows the problem
> loop.exit:                                        ; preds =
> %loop.body, %loop.body.2
>   store i32 %inc.i7 <-- %inc.i7 is the wrong value because
> AA mistakenly compute the GEP due to sext the i1 true.
> 
> Please take a look.
> 
> Thanks,
> 
> Yin
> -----Original Message-----
> From: Hal Finkel [mailto:hfinkel at anl.gov]
> Sent: Tuesday, December 09, 2014 10:21 AM
> To: Yin Ma
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: Wrong value for sext i1 true in BasicAliasAnalysis
> 
> Hi Yin,
> 
> Do you have a test case for this? I don't understand where this
> special case comes from. And if we do have a special case here, it
> seems this should be noted in the LangRef (which currently says
> only, "If the offsets have a different width from the pointer, they
> are sign-extended or truncated to the width of the pointer."). And,
> in fact, the LangRef documentation for the sext instructions says,
> "When sign extending from i1, the extension always results in -1 or
> 0."
> 
> Thanks again,
> Hal
> 
> ----- Original Message -----
> > From: "Yin Ma" <yinma at codeaurora.org>
> > To: llvm-commits at cs.uiuc.edu
> > Cc: hfinkel at anl.gov
> > Sent: Monday, December 8, 2014 4:49:34 PM
> > Subject: Wrong value for sext i1 true in BasicAliasAnalysis
> > 
> > 
> > 
> > 
> > Hi Hal,
> > 
> > 
> > 
> > How are you doing. For your patch db1e51359, you changed offset
> > computation
> > 
> > from zext to sext. However, I came across a case that expression
> > contains i1 true to i32.
> > 
> > true is value 1 for offset. if it is sext to 32bit, the result of
> > sext
> > will not be 1. Instead, it will
> > 
> > be -1. Sext true is a special case. This will make AA check failed
> > when one GEP contains
> > 
> > i1 true to i32 expression but the other GEP doesn’t.
> > 
> > 
> > 
> > I attached my change. How do you think? If it is valid patch,
> > please
> > merge it to the tip.
> > 
> > 
> > 
> > Thanks,
> > 
> > 
> > 
> > Yin
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
A non-text attachment was scrubbed...
Name: q-i8.ll
Type: application/octet-stream
Size: 1653 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141214/8ef0fcbd/attachment.obj>