[llvm-commits] [llvm] r74964 - in /llvm/trunk: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp test/CodeGen/X86/vec_compare.ll

[Dan Gohman]

On Jul 7, 2009, at 9:10 PM, Eli Friedman wrote:


> On Tue, Jul 7, 2009 at 9:09 PM, Eli Friedman<eli.friedman at gmail.com>  
> wrote:
>
>> On Tue, Jul 7, 2009 at 7:38 PM, Eli  
>> Friedman<eli.friedman at gmail.com> wrote:
>>
>>> On Tue, Jul 7, 2009 at 7:16 PM, Eli  
>>> Friedman<eli.friedman at gmail.com> wrote:
>>>
>>>> On Tue, Jul 7, 2009 at 6:29 PM, Dan Gohman<gohman at apple.com> wrote:
>>>>
>>>>> EXTRACT_SUBVECTOR is normally used to extract a legal value from
>>>>>
>>>>> an illegal one. The code above does the opposite, extracting a
>>>>>
>>>>> potentially illegal value from a legal one.
>>>>>
>>>>
>>>>
>>>> As far as I can tell, we support both cases properly in type
>>>>
>>>> legalization; what specific case breaks?
>>>>
>>>
>>>
>>> Mmm, nevermind, you're talking about something like the following?
>>>
>>>
>>>
>>> define void @test2(<2 x i32> %A, <2 x i32> %B, <2 x i32>* %C)  
>>> nounwind {
>>>
>>>       %D = icmp sgt <2 x i32> %A, %B
>>>
>>>       %E = zext <2 x i1> %D to <2 x i32>
>>>
>>>       store <2 x i32> %E, <2 x i32>* %C
>>>
>>>       ret void
>>>
>>> }
>>>
>>>
>>>
>>> It's really just falling apart because we don't support various
>>>
>>> related cases correctly; first, it hits the "Unimp" assert in this
>>>
>>> patch,  and if you just comment it out (which seems like the right
>>>
>>> thing to do), we hit an assertion because we don't support  
>>> extracting
>>>
>>> an i1 from a <2 x i1> in PromoteIntRes_EXTRACT_VECTOR_ELT .  Still
>>>
>>> nothing going wrong with EXTRACT_SUBVECTOR.
>>>
>>
>>
>> Attached partially deals with the issues (enough to get the given
>>
>> testcase to compile); it might not be the best approach, though,
>>
>> because changing the definition of EXTRACT_VECTOR_ELT has side- 
>> effects
>>
>> all over the place.

This change to EXTRACT_VECTOR_ELT seems like a good idea -- it nicely
mirrors INSERT_VECTOR_ELT, and the ARM target may have a use for it.
The main question is what type of extension it does -- sign, zero, or
any? It appears the answer is currently any, though that doesn't sound
correct here, since i1 values promoted to i8 are usually required to
be zero-extended. Would it make sense to define EXTRACT_VECTOR_ELT as
explicitly using zero-extension?

For the ARM target, we could then define a separate
EXTRACT_VECTOR_ELT_SEXT, as it needs both.

Dan