[llvm-commits] [llvm-gcc-4.2] r48608 - in /llvm-gcc-4.2/trunk/gcc: config/i386/llvm-i386-target.h config/i386/llvm-i386.cpp config/rs6000/llvm-rs6000.cpp config/rs6000/rs6000.h llvm-abi.h llvm-convert.cpp

[Duncan Sands]

Hi Dale, while this is better it is not there yet.

> -  //EmitCall(exp, DestLoc);
> -  Value *Result = EmitCallOf(Callee, exp, DestLoc, PAL);
> +  Value *Result;
> +  if (TREE_CODE(TREE_TYPE(exp))==VECTOR_TYPE &&
> +      LLVM_SHOULD_RETURN_VECTOR_AS_SHADOW(TREE_TYPE(exp),
> +                                fndecl ? DECL_BUILT_IN(fndecl) : false)) {
> +    // Vectors are first class types, so we need to return a value,
> +    // but this one is to be passed using sret.  Pass in a DestLoc, if
> +    // there isn't one already, and return a Load from it as the value.
> +    if (!DestLoc) {
> +      MemRef NewLoc = CreateTempLoc(ConvertType(TREE_TYPE(exp)));
> +      DestLoc = &NewLoc;
> +    }
> +    EmitCallOf(Callee, exp, DestLoc, PAL);
> +    LoadInst *LI = Builder.CreateLoad(DestLoc->Ptr, false, "tmp");
> +    Result = LI;
> +  } else {
> +    //EmitCall(exp, DestLoc);
> +    Result = EmitCallOf(Callee, exp, DestLoc, PAL);
> +  }

This should be taken care of entirely inside EmitCallOf.  EmitCallOf should
return the vector call result as a Value.  What is more, the whole point of
having an ABIClient is that all mucking around with ABI conventions is encapsulated
in one place, rather than spreading the logic around as you are doing.
The way EmitCallOf finds out it is pass-by-sret is by
HandleAggregateShadowArgument being called, not by performing the kinds
of test you inserted above.  If done right, then it (the EmitCallOf logic)
will automatically be able to handle cases such as: integer/float being
passed-by-sret, if one day the ABI logic decides to call HandleAggregateShadowArgument
for such a guy.

> -      bool isSigned = !TYPE_UNSIGNED(TREE_TYPE(TREE_VALUE(arg)));
> -      CallOperands.push_back(CastToAnyType(V, isSigned, ArgTy, false));
> +      if (TREE_CODE(TREE_TYPE(TREE_VALUE(arg)))==VECTOR_TYPE &&
> +          LLVM_SHOULD_PASS_VECTOR_IN_INTEGER_REGS(TREE_TYPE(TREE_VALUE(arg)))) {
> +        // Passed as integer registers.  We need a stack object, not a value.
> +        MemRef NewLoc = CreateTempLoc(ConvertType(TREE_TYPE(TREE_VALUE(arg))));
> +        StoreInst *St = Builder.CreateStore(V, NewLoc.Ptr, false);
> +        Client.setLocation(NewLoc.Ptr);
> +        ParameterAttributes Attributes = ParamAttr::None;
> +        ABIConverter.HandleArgument(TREE_TYPE(TREE_VALUE(arg)), &Attributes);
> +        if (Attributes != ParamAttr::None)
> +          PAL = PAL.addAttr(CallOperands.size(), Attributes);
> +      } else {
> +        bool isSigned = !TYPE_UNSIGNED(TREE_TYPE(TREE_VALUE(arg)));
> +        CallOperands.push_back(CastToAnyType(V, isSigned, ArgTy, false));
> +      }

This also looks bogus - this kind of decision should be made in the generic
ABI stuff, not here.  That said, the existing parameter passing logic looks
rather bogus to me too.  I am less familiar with the parameter passing part,
so I will have to think about it a bit.

Ciao,

Duncan.