llvm dev - Jun 2015 - [LLVMdev] extractelement causes memory access violation - what to do?

On Fri, Jun 26, 2015 at 5:42 PM David Majnemer <david.majnemer at
gmail.com>
wrote:
> On Fri, Jun 26, 2015 at 7:00 AM, Paweł Bylica <chfast at gmail.com>
wrote:
>
>> Hi,
>>
>> Let's have a simple program:
>> define i32 @main(i32 %n, i64 %idx) {
>>   %idxSafe = trunc i64 %idx to i5
>>   %r = extractelement <4 x i32> <i32 -1, i32 -1, i32 -1, i32
-1>, i64 %idx
>>   ret i32 %r
>> }
>>
>> The assembly of that would be:
>> pcmpeqd %xmm0, %xmm0
>> movdqa %xmm0, -24(%rsp)
>> movl -24(%rsp,%rsi,4), %eax
>> retq
>>
>> The language reference states that the extractelement instruction
>> produces undefined value in case the index argument is invalid (our
case).
>> But the implementation simply dumps the vector to the stack memory,
>> calculates the memory offset out of the index value and tries to access
the
>> memory. That causes the crash.
>>
>> The workaround is to trunc the index value before extractelement (see
>> %idxSafe). But what should be the ultimate solution?
>>
>
> We could fix this by specifying that out of bounds access on an
> extractelement leads to full-on undefined behavior, no need to force
> everyone to eat the cost of a mask.
>
I don't have preference for any of the solutions.

I have a side question. It is not stated explicitly in the reference but I
would assume the index of extractelement is processed as an unsigned value.
However, the DAG Builder extends the index with sext. Is it correct?

- PB
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150630/d2334fe2/attachment.html>

----- Original Message -----
> From: "Paweł Bylica" <chfast at gmail.com>
> To: "David Majnemer" <david.majnemer at gmail.com>
> Cc: "LLVMdev" <llvmdev at cs.uiuc.edu>
> Sent: Tuesday, June 30, 2015 5:42:24 AM
> Subject: Re: [LLVMdev] extractelement causes memory access violation - what
to do?
> 
> 
> 
> 
> 
> On Fri, Jun 26, 2015 at 5:42 PM David Majnemer <
> david.majnemer at gmail.com > wrote:
> 
> 
> 
> 
> 
> On Fri, Jun 26, 2015 at 7:00 AM, Paweł Bylica < chfast at gmail.com >
> wrote:
> 
> 
> 
> Hi,
> 
> 
> Let's have a simple program:
> 
> define i32 @main(i32 %n, i64 %idx) {
> %idxSafe = trunc i64 %idx to i5
> %r = extractelement <4 x i32> <i32 -1, i32 -1, i32 -1, i32 -1>,
i64
> %idx
> ret i32 %r
> }
> 
> 
> The assembly of that would be:
> 
> pcmpeqd %xmm0, %xmm0
> movdqa %xmm0, -24(%rsp)
> movl -24(%rsp,%rsi,4), %eax
> retq
> 
> 
> The language reference states that the extractelement instruction
> produces undefined value in case the index argument is invalid (our
> case). But the implementation simply dumps the vector to the stack
> memory, calculates the memory offset out of the index value and
> tries to access the memory. That causes the crash.
> 
> 
> The workaround is to trunc the index value before extractelement (see
> %idxSafe). But what should be the ultimate solution?
> 
> 
> 
> 
> 
> We could fix this by specifying that out of bounds access on an
> extractelement leads to full-on undefined behavior, no need to force
> everyone to eat the cost of a mask.
> 
> 
> I don't have preference for any of the solutions.
> 
> 
> I have a side question. It is not stated explicitly in the reference
> but I would assume the index of extractelement is processed as an
> unsigned value. However, the DAG Builder extends the index with
> sext. Is it correct?
Hrmm. Given that only (small) positive numbers are valid, it shouldn't
matter. Unless we can find a reason that it works better to be sext, it seems
conceptually cleaner to make it zext.

 -Hal
> 
> 
> - PB
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory

On Tue, Jun 30, 2015 at 11:03 PM Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
> > From: "Paweł Bylica" <chfast at gmail.com>
> > To: "David Majnemer" <david.majnemer at gmail.com>
> > Cc: "LLVMdev" <llvmdev at cs.uiuc.edu>
> > Sent: Tuesday, June 30, 2015 5:42:24 AM
> > Subject: Re: [LLVMdev] extractelement causes memory access violation -
> what   to do?
> >
> >
> >
> >
> >
> > On Fri, Jun 26, 2015 at 5:42 PM David Majnemer <
> > david.majnemer at gmail.com > wrote:
> >
> >
> >
> >
> >
> > On Fri, Jun 26, 2015 at 7:00 AM, Paweł Bylica < chfast at gmail.com
>
> > wrote:
> >
> >
> >
> > Hi,
> >
> >
> > Let's have a simple program:
> >
> > define i32 @main(i32 %n, i64 %idx) {
> > %idxSafe = trunc i64 %idx to i5
> > %r = extractelement <4 x i32> <i32 -1, i32 -1, i32 -1, i32
-1>, i64
> > %idx
> > ret i32 %r
> > }
> >
> >
> > The assembly of that would be:
> >
> > pcmpeqd %xmm0, %xmm0
> > movdqa %xmm0, -24(%rsp)
> > movl -24(%rsp,%rsi,4), %eax
> > retq
> >
> >
> > The language reference states that the extractelement instruction
> > produces undefined value in case the index argument is invalid (our
> > case). But the implementation simply dumps the vector to the stack
> > memory, calculates the memory offset out of the index value and
> > tries to access the memory. That causes the crash.
> >
> >
> > The workaround is to trunc the index value before extractelement (see
> > %idxSafe). But what should be the ultimate solution?
> >
> >
> >
> >
> >
> > We could fix this by specifying that out of bounds access on an
> > extractelement leads to full-on undefined behavior, no need to force
> > everyone to eat the cost of a mask.
> >
> >
> > I don't have preference for any of the solutions.
> >
> >
> > I have a side question. It is not stated explicitly in the reference
> > but I would assume the index of extractelement is processed as an
> > unsigned value. However, the DAG Builder extends the index with
> > sext. Is it correct?
>
> Hrmm. Given that only (small) positive numbers are valid, it shouldn't
> matter. Unless we can find a reason that it works better to be sext, it
> seems conceptually cleaner to make it zext.
>
I have tried to change it to zext. 2 Mips test have failed. I haven't
checked the details though.
sext looks pretty wrong to me because i5 -1 does not mean 31 any more.

- PB

>
>  -Hal
>
> >
> >
> > - PB
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150630/e93e7658/attachment.html>