[PATCH] Make Scalar Replacement of Aggregates use the right opcode for converting from vector-of-int to vector-of-pointer

[Tom Roeder]

Here's an updated patch with three tests (I found more problems as I
worked on minimizing my test case) and a check added to
canConvertValue. Note that AFAICT, I can't just strip the vector type
then check compatibility of the conversion, since (as one of my tests
shows) there's a problem that <1 x i32*> can't be converted to i64,
while the stripped version of the check would try to turn it into a
ptrtoint.

Please let me know what you think.

Tom

On Fri, Sep 13, 2013 at 1:51 PM, Tom Roeder <tmroeder at google.com> wrote:
> Actually, I just figured it out. It's probably not minimal yet, but it does
> trigger the crash:
>
> define <4 x i64> @f({<2 x i32*>, <2 x i32*>} %x) {
>   %a = alloca {<2 x i32*>, <2 x i32*>}
>   store {<2 x i32*>, <2 x i32*>} %x, {<2 x i32*>, <2 x i32*>}* %a
>   %cast = bitcast {<2 x i32*>, <2 x i32*>}* %a to <4 x i64>*
>   %vec = load <4 x i64>* %cast
>   ret <4 x i64> %vec
> }
>
> On Fri, Sep 13, 2013 at 1:16 PM, Tom Roeder <tmroeder at google.com> wrote:
>> Sorry, I don't follow your description; I don't know how the SROA
>> algorithm works on vectors, so I don't know the conditions under which
>> I can get SROA to convert GEP extraction over vectors to vector loads
>> and stores. And I haven't yet grokked the code. Under what
>> circumstances would SROA do these conversions?
>>
>> On Thu, Sep 12, 2013 at 3:20 PM, Chandler Carruth <chandlerc at google.com> wrote:
>>> On Thu, Sep 12, 2013 at 2:54 PM, Tom Roeder <tmroeder at google.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> In the process of trying to compile Chromium with clang -flto -O3,
>>>> I've run into a crash in SROA;  the pass is trying to cast <2 x i64>
>>>> to <2 x %"struct.webrtc::WindowCapturer::Window"*> with a bitcast,
>>>> when it should be using inttoptr. I believe this patch fixes the
>>>> problem.
>>>>
>>>> The problem is that convertValue in lib/Transforms/Scalar/SROA.cpp is
>>>> being called to convert the int vector into the pointer vector, and it
>>>> falls through all the cases to the bitcast at the end. So, this
>>>> patches catches that case and outputs the correct opcode.
>>>>
>>>> However, I'm not really familiar with either the code itself or the
>>>> SROA algorithm, so there might be something wrong with this patch. It
>>>> fixes my build, and all tests pass in the test suite, but I haven't
>>>> yet been able to extract a minimal test case from the crash I observe
>>>> in the massive Chromium LTO build that I've been working on.
>>>>
>>>> I'm hoping that this description of the crash and the fix will be
>>>> enough for someone to point me in the right direction for creating a
>>>> simple test case that triggers it.
>>>
>>>
>>> You should be able to create a simple test by writing code that extracts
>>> vector elements with GEPs and getting SROA to convert them to vector loads
>>> and stores. Then you just want to get one of them to use pointer types
>>> internally and the others to use integers, forcing the value conversion.
>>>
>>> You'll likely need a corresponding change to canConvertValue to support
>>> vector types.
>>>
>>> You should handle this by stripping the vector-ness first, and then handling
>>> the element type, so that non-vector types and vector element types have the
>>> same code.
>>>
>>>>
>>>>
>>>> Tom
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sroa_vector.patch
Type: application/octet-stream
Size: 3074 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130913/639c5852/attachment.obj>