[llvm-commits] [llvm-gcc-4.2] r48502 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

[Duncan Sands]

Hi Dale,

>  Value *TreeToLLVM::Emit(tree exp, const MemRef *DestLoc) {
> -  assert((isAggregateTreeType(TREE_TYPE(exp)) == (DestLoc != 0) ||
> +  tree fndecl;
> +  // Some vectors are returned using sret; these should have DestLoc provided.
> +  assert(((isAggregateTreeType(TREE_TYPE(exp)) == (DestLoc != 0)) ||
> +          (TREE_CODE(exp)==CALL_EXPR && 
> +           TREE_CODE(TREE_TYPE(exp))==VECTOR_TYPE &&
> +           LLVM_SHOULD_RETURN_VECTOR_AS_SHADOW(TREE_TYPE(exp),
> +             ((fndecl = get_callee_fndecl(exp)) ? DECL_BUILT_IN(fndecl) : 
> +                                                false))) ||
>            TREE_CODE(exp) == MODIFY_EXPR) &&

rather than this ugliness in Emit, how about teaching the return value code to
handle it, by having it only introduce the DestLoc+Emit-to-DestLoc when handling
an aggregate, and making it generate an Emit-of-scalar+Store for a scalar (in
this case a vector type).  This is how everywhere else handles the scalar vs
aggregate distinction.

> -
> -    Value *RHS = Emit(rhs, 0);
> +    // Some types (so far, generic vectors on x86-32 and ppc32) can have
> +    // GCC SSA temporaries for them, but are passed using sret, which means
> +    // Emit will return 0.  Handle this.

Presumably Emit is only returning 0 because it was provided with DestLoc.  So
if you make the modification I requested above, the code here presumably would
no longer be needed.

> +    Value *RHS;
> +    tree fndecl;
> +    if (TREE_CODE(rhs)==CALL_EXPR && 
> +        TREE_CODE(TREE_TYPE(rhs))==VECTOR_TYPE &&
> +        LLVM_SHOULD_RETURN_VECTOR_AS_SHADOW(TREE_TYPE(rhs), 
> +          ((fndecl = get_callee_fndecl(rhs)) ? DECL_BUILT_IN(fndecl) : 
> +                                               false))) {
> +      bool isVolatile = TREE_THIS_VOLATILE(lhs);
> +      unsigned Alignment = expr_align(lhs) / 8;
> +      MemRef NewLoc = CreateTempLoc(ConvertType(TREE_TYPE(rhs)));
> +      Emit(rhs, &NewLoc);
> +      LoadInst *LI = Builder.CreateLoad(NewLoc.Ptr, isVolatile, "tmp");
> +      LI->setAlignment(Alignment);
> +      RHS = LI;
> +    } else {
> +      RHS = Emit(rhs, 0);
> +    }
>      RHS = CastToAnyType(RHS, RHSSigned, ConvertType(TREE_TYPE(lhs)), LHSSigned);
>      SET_DECL_LLVM(lhs, RHS);
>      return RHS;

Please don't break Emit invariants, it is only asking for trouble.

Ciao,

Duncan.