[llvm-commits] [llvm] r82300 - in /llvm/trunk/lib/Transforms: IPO/FunctionAttrs.cpp IPO/GlobalOpt.cpp Scalar/GVN.cpp Scalar/InstructionCombining.cpp Scalar/Reassociate.cpp Scalar/SCCP.cpp Scalar/SimplifyLibCalls.cpp Scalar/TailDuplication.cpp Utils/InlineCost.cpp

[Török Edwin]

On 2009-09-19 01:35, Victor Hernandez wrote:
> Author: hernande
> Date: Fri Sep 18 17:35:49 2009
> New Revision: 82300
>
> URL: http://llvm.org/viewvc/llvm-project?rev=82300&view=rev
> Log:
> Enhance transform passes so that they apply the same tranforms to malloc calls as to MallocInst.
>
> Reviewed by Dan Gohman.
>
>
> +        if (!isa<ICmpInst>(LoadUse.getUser()))
> +          LoadUse = RepValue;
> +        else {
> +          ICmpInst *ICI = cast<ICmpInst>(LoadUse.getUser());
>   

Can this condition be inverted to use dyn_cast instead of !isa and cast?

> +  // The tricky aspect of this transformation is handling the case when malloc
> +  // fails.  In the original code, malloc failing would set the result pointer
> +  // of malloc to null.  In this case, some mallocs could succeed and others
> +  // could fail.  As such, we emit code that looks like this:
> +  //    F0 = malloc(field0)
> +  //    F1 = malloc(field1)
> +  //    F2 = malloc(field2)
> +  //    if (F0 == 0 || F1 == 0 || F2 == 0) {
> +  //      if (F0) { free(F0); F0 = 0; }
> +  //      if (F1) { free(F1); F1 = 0; }
> +  //      if (F2) { free(F2); F2 = 0; }
> +  //    }
>   

Those 3 ifs are not needed, doing free(NULL) is perfectly fine, so it
could look like this:

    F0 = malloc(field0);
    F1 = malloc(field1);
    F2 = malloc(field2);
    if (F0 == 0 || F1 == 0 || F2 == 0) {
           free(F0); free(F1); free(F2);
           F0 = 0; F1 = 0; F2 = 0;
    }


If only one malloc fails then 3 frees will be called instead of 1, but
malloc failing is an exceptional condition,
so the time wasted by those extra 2 function calls don't matter. Instead
by removing those 3 ifs you reduce code size.

Best regards,
--Edwin