[llvm-commits] [llvm] r51468 - in /llvm/trunk: include/llvm/Bitcode/LLVMBitCodes.h include/llvm/Type.h lib/AsmParser/LLLexer.cpp lib/AsmParser/llvmAsmParser.cpp.cvs lib/AsmParser/llvmAsmParser.h.cvs lib/AsmParser/llvmAsmParser.y lib/AsmParser/llvmAsmParser.y.cvs lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Writer/BitcodeWriter.cpp lib/Bitcode/Writer/ValueEnumerator.cpp test/Assembler/insertextractvalue.ll test/Verifier/2006-07-11-StoreStruct.ll utils/llvm.grm

[Chris Lattner]

On May 22, 2008, at 6:55 PM, Dan Gohman wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=51468&view=rev
> Log:
> Make structs and arrays first-class types, and add assembly
> and bitcode support for the extractvalue and insertvalue
> instructions and constant expressions.

Woo!

> +++ llvm/trunk/include/llvm/Type.h Thu May 22 20:55:30 2008
> @@ -216,8 +216,8 @@
>   /// is a valid type for a Value.
>   ///
>   inline bool isFirstClassType() const {
> -    // Coming soon: first-class struct and array types...
> -    return isSingleValueType();
> +    return isSingleValueType() ||
> +           ID == StructTyID || ID == ArrayTyID;
>   }

At this point, is it faster to just check that this *isn't* void and  
functiontype?

> @@ -1966,6 +1967,48 @@
>       GEN_ERROR("Invalid shufflevector operands");
>     $$ = ConstantExpr::getShuffleVector($3, $5, $7);
>     CHECK_FOR_ERROR
> +  }
> +  | EXTRACTVALUE '(' ConstVal IndexList ')' {

Does IndexList start with a comma?  "extractvalue ( {i32} zeroinit  
0 )" is strange, I think a comma is needed.  Also, constantexprs  
should not require types on the indices either.  It should just take a  
list of EUINT64VAL's, like getresult.

> +++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp Thu May 22  
> 20:55:30 2008
> @@ -103,10 +103,10 @@
>   // in the table (have low bit-width).
>   std::stable_sort(Types.begin(), Types.end(), CompareByFrequency);
>
> -  // Partition the Type ID's so that the first-class types occur  
> before the
> +  // Partition the Type ID's so that the single-value types occur  
> before the
>   // aggregate types.  This allows the aggregate types to be dropped  
> from the
>   // type table after parsing the global variable initializers.
> -  std::partition(Types.begin(), Types.end(), isFirstClassType);
> +  std::partition(Types.begin(), Types.end(), isSingleValueType);

Is this required for correctness?  This could cause a significant BC  
file size increase for real world apps.  I'd try kimwitu and 176.gcc  
as examples.

Thanks for working on this Dan!

-Chris