Wrong value for sext i1 true in BasicAliasAnalysis
Hal Finkel
hfinkel at anl.gov
Sun Dec 14 10:48:04 PST 2014
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>
More information about the llvm-commits
mailing list