[llvm-bugs] [Bug 32323] New: Potential incorrect generation of store-pair

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=32323

            Bug ID: 32323
           Summary: Potential incorrect generation of store-pair
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Backend: AArch64
          Assignee: unassignedbugs at nondot.org
          Reporter: pirama at google.com
                CC: llvm-bugs at lists.llvm.org

Created attachment 18119
  --> https://bugs.llvm.org/attachment.cgi?id=18119&action=edit
files to reproduce issue

This is an issue reported internally within Android.  At a high level, until
r276451, two stores incorrectly got merged into a 'stp' despite there being an
aliasing write between the first store and the merged 'stp'.  This issue goes
away with r276452
(http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160718/375442.html).
 But it's not clear if this revision indirectly fixes the underlying issue or
if it just masks a bug in the backend.

Here's the original bug report:
===============================

This turned up compiling vulkan validation layer tests.
Some tests had bad fields in structs despite source code that initialized them.
The reduced test case shows the fields being initialized and then overwritten.
The problem can be masked by explicitly initializing another field.

These assignments are implemented as a store of 7 into a stack address at
[sp,#44] and 1 into [sp,#48].

    descriptorset_layout_binding.descriptorType =
VK_DESCRIPTOR_TYPE_STORAGE_BUFFER;
    descriptorset_layout_binding.descriptorCount = 1;

The problem can be masked by explicitly initializing another field in line 120.
That changes the code generated for line 126 and line 115.

115     VkDescriptorSetLayoutBinding descriptorset_layout_binding = {};
116     descriptorset_layout_binding.descriptorType =
VK_DESCRIPTOR_TYPE_STORAGE_BUFFER;
117     descriptorset_layout_binding.descriptorCount = 1;
118     descriptorset_layout_binding.stageFlags = VK_SHADER_STAGE_ALL;
119 #ifdef FIX
120     descriptorset_layout_binding.pImmutableSamplers = nullptr;
121 #endif
122 
123     VkDescriptorSetLayoutCreateInfo descriptorset_layout_create_info = {};
124     descriptorset_layout_create_info.sType =
VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO;
125     descriptorset_layout_create_info.bindingCount = 1;
126     descriptorset_layout_create_info.pBindings =
&descriptorset_layout_binding;

The attached testit.sh script shows the difference between code compiled with
and without the workaround.
The diff makes it a little easier to see what goes wrong.
The value at [sp,#44] is overwritten with zero by a stp x8,xzr,[sp,#32].
That instruction is supposed to be writing to [sp,#32] in a neighboring struct.
It seems to also be trying to zero out four bytes at [sp,#40].  But it clobbers
[sp,#44].

    descriptorset_layout_create_info.pBindings = &descriptorset_layout_binding;

-       .loc    1 126 48                // t.cpp:126:48
-       str     x8, [sp, #32]
+       .loc    1 115 34 discriminator 1 // t.cpp:115:34
+       stp     x8, xzr, [sp, #32]

The version with workaround is marked as "-".
The broken version is marked as "+".

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20170317/aff30459/attachment.html>