[llvm-commits] [llvm] r58964 - in /llvm/trunk: docs/LangRef.html lib/Bitcode/Reader/BitcodeReader.cpp lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/CodeGen/SelectionDAG/LegalizeTypes.h lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp lib/Transforms/Scalar/InstructionCombining.cpp lib/VMCore/ConstantFold.cpp lib/VMCore/Instructions.cpp lib/VMCore/Verifier.cpp

[Chris Lattner]

On Nov 16, 2008, at 3:53 PM, Mon Ping Wang wrote:

> Hi Chris,
>
> I took some .ll files that used shuffles and created bc files using  
> llvm 2.4 llvm-as  and run them through a 2.4 opt and then called a  
> TOT llc  on the llvm-as and opt generated bytecode files to generate  
> assembly.  For the limited number of files that I tried, I didn't  
> have any problems.  Is there some other experiment I should be doing?

Yep, that is the right experiment.  Looking at getValueTypePair(), I  
see that in some cases it encodes to just one value (when 'ValNo <  
InstNum').  Is this condition guaranteed to always be true?  If so,  
this patch looks great.

One other minor thing, does this generalize constantexpr shuffleexprs  
to match (CST_CODE_CE_SHUFFLEVEC)?

-Chris

>
>
> Thanks,
>  -- Mon Ping
>
>
> On Nov 15, 2008, at 10:29 PM, Chris Lattner wrote:
>
>> On Nov 9, 2008, at 8:46 PM, Mon P Wang wrote:
>>> URL: http://llvm.org/viewvc/llvm-project?rev=58964&view=rev
>>> Log:
>>> Added support for the following definition of shufflevector
>>> <result> = shufflevector <n x <ty>> <v1>, <n x <ty>> <v2>, <m x  
>>> i32> <mask>
>>
>> Very nice Mon Ping!
>>
>> One import comment though:
>>
>>> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Sun Nov  9  
>>> 22:46:22 2008
>>> @@ -1592,16 +1592,12 @@
>>>         getValue(Record, OpNum, Vec1->getType(), Vec2))
>>>       return Error("Invalid SHUFFLEVEC record");
>>>
>>> -      const Type *MaskTy =
>>> -        VectorType::get(Type::Int32Ty,
>>> -                        cast<VectorType>(Vec1->getType())- 
>>> >getNumElements());
>>> -
>>> -      if (getValue(Record, OpNum, MaskTy, Mask))
>>> +      if (getValueTypePair(Record, OpNum, NextValueNo, Mask))
>>>       return Error("Invalid SHUFFLEVEC record");
>>>     I = new ShuffleVectorInst(Vec1, Vec2, Mask);
>>>     break;
>>>   }
>>
>> AFAICT, this will break compatibility with LLVM 2.4 bc files.  This  
>> isn't good.  It should be possible to check the number of operands  
>> in Record.  If there are N+1 entries, it is the new format, if  
>> there are N, it is the old format.  Please verify that you can read  
>> 2.4 (or earlier) bc files with shuffles in them.
>>
>> -Chris
>