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

[Tom Roeder]

Ping

On Fri, Sep 13, 2013 at 4:43 PM, Tom Roeder <tmroeder at google.com> wrote:
> 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
>>>>>
>>>>