llvm dev - Aug 2015 - load instruction erroneously removed by GVN

On 08/07/2015 01:53 PM, Caldarale, Charles R wrote:
>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org]
>> On Behalf Of Mikael Holmén via llvm-dev
>> Subject: [llvm-dev] load instruction erroneously removed by GVN
>
>> But between the load and the alloca there is also
>>     call fastcc void @format_long(i16* %_tmp30, i16 10, i32 10), !dbg
!22
>> which will use %_tmp30 to write in the alloca'd buffer.
>
>> Shoulnd't MemoryDependenceAnalysis::getDependency rather return the
call?
>
> Depends.  What is the exact declaration of format_long?
In the input .ll file it is:

; Function Attrs: minsize optsize
define internal i16 @format_long(i16* %res.8.par, i16 %base.9.par, i32 
%x.10.par) #3 {

which is later changed somewhere in opt to:

; Function Attrs: minsize nounwind optsize
define internal fastcc i16 @format_long(i16* %res.8.par, i16 
%base.9.par, i32 %x.10.par) #2 {

Thanks,
Mikael
>
>   - Chuck
>
>
>

I noticed that the alloca is marked as "align 1".  Could it be that
the store in @format_long has a coarser alignment (align 4 store to an
align 1 alloca is UB).  Note that the default alignment on loads and
stores is ABI alignment, not 1.

Otherwise, this looks like a legitimate problem to me, please file a
bug with a reproducer.

-- Sanjoy

On 7 August 2015 at 05:02, Mikael Holmén <llvm-dev at lists.llvm.org>
wrote:
>
>
> On 08/07/2015 01:53 PM, Caldarale, Charles R wrote:
>
>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org]
>>> On Behalf Of Mikael Holmén via llvm-dev
>>> Subject: [llvm-dev] load instruction erroneously removed by GVN
>>>
>>
>> But between the load and the alloca there is also
>>>     call fastcc void @format_long(i16* %_tmp30, i16 10, i32 10),
!dbg !22
>>> which will use %_tmp30 to write in the alloca'd buffer.
>>>
>>
>> Shoulnd't MemoryDependenceAnalysis::getDependency rather return the
call?
>>>
>>
>> Depends.  What is the exact declaration of format_long?
>>
>
> In the input .ll file it is:
>
> ; Function Attrs: minsize optsize
> define internal i16 @format_long(i16* %res.8.par, i16 %base.9.par, i32
> %x.10.par) #3 {
>
> which is later changed somewhere in opt to:
>
> ; Function Attrs: minsize nounwind optsize
> define internal fastcc i16 @format_long(i16* %res.8.par, i16 %base.9.par,
> i32 %x.10.par) #2 {
>
That's part of it, but we also need to see what #3 and #2 refer to in this
context. Those should be grouped at the end of your .ll, something like

attributes #3 = { nobuiltin nounwind
"less-precise-fpmad"="false"
"no-frame-pointer-elim"="false"
"no-frame-pointer-elim-non-leaf"
"no-infs-fp-math"="false"
"no-nans-fp-math"="false"
"stack-protector-buffer-size"="8"
"unsafe-fp-math"="false"
"use-soft-float"="false" }

or

attributes #2 = { nounwind readonly }

If @format_long is marked with the 'readonly' or 'readnone'
attribute then
GVN would be justified in doing the transformation you've described.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150807/61eee1ac/attachment.html>

Hi,

On 08/07/2015 10:30 PM, Nick Lewycky wrote:
[...]
>         Depends.  What is the exact declaration of format_long?
>
>
>     In the input .ll file it is:
>
>     ; Function Attrs: minsize optsize
>     define internal i16 @format_long(i16* %res.8.par, i16 %base.9.par,
>     i32 %x.10.par) #3 {
>
>     which is later changed somewhere in opt to:
>
>     ; Function Attrs: minsize nounwind optsize
>     define internal fastcc i16 @format_long(i16* %res.8.par, i16
>     %base.9.par, i32 %x.10.par) #2 {
>
>
> That's part of it, but we also need to see what #3 and #2 refer to in
> this context. Those should be grouped at the end of your .ll, something
like
>
> attributes #3 = { nobuiltin nounwind
"less-precise-fpmad"="false"
> "no-frame-pointer-elim"="false"
"no-frame-pointer-elim-non-leaf"
> "no-infs-fp-math"="false"
"no-nans-fp-math"="false"
> "stack-protector-buffer-size"="8"
"unsafe-fp-math"="false"
> "use-soft-float"="false" }
The attributes are

attributes #3 = { minsize optsize "target-cpu"="phoenixIV" }

and

attributes #2 = { minsize nounwind optsize
"target-cpu"="phoenixIV" }

(I'm compiling for my out-of-tree target, thus the 
"target-cpu"="phoenixIV").

So no readonly or readnone attributes.

Thanks,
Mikael
>
> or
>
> attributes #2 = { nounwind readonly }
>
> If @format_long is marked with the 'readonly' or 'readnone'
attribute
> then GVN would be justified in doing the transformation you've
described.
>
> Nick