[llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr

[Jin Gu Kang]

Hi Duncan

I forgot to ask a question about wrong use count.

Making ConstantExpr for GetElementPtr is following:
  Constant *CreateInBoundsGetElementPtr(Constant *C, Value* const *IdxList,
                                        unsigned NumIdx) const {
    return Fold(ConstantExpr::getInBoundsGetElementPtr(C, IdxList, NumIdx));
  }

When argument 'C' is bitcast ConstantExpr on above code, 
SymbolicallyEvaluateGEP() function under Fold() function changes bitcast ConstantExpr
to bitcast ConstantExpr's operand(0) using stripPointerCasts() and makes new GEP using it
This increases use count of bitcast ConstantExpr's operand(0).
(I tested that bitcast ConstantExpr's operand(0) is global variable for struct type with initializer.)
If some passes use information of use count for value, wrong use count may cause wrong pass.

To solve this issue, I thought that if first argument of GetElementPtr ConstantExpratter is bitcast
ConstantExpr and it is useless after making GetElementPtr ConstantExpr, it has to be removed
because of wrong use count.

what do you think about wrong use count?

P.S 
llvm passes which remove useless ConstantExpr are likely to be often called before each pass which uses
information of use count.

Thanks,
Jin-Gu Kang
________________________________________
From: Duncan Sands [baldrick at free.fr]
Sent: Tuesday, February 22, 2011 6:22 PM
To: Jin Gu Kang
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr

Hi Jin Gu Kang,

> OK. I unstand :)
>
> I send modified patch

I think this is pointless.  Unused constants can be created in many places, so
concentrating on one makes no sense.  If you want to get rid of them it would
be better to add a pass that looks at all constants and zaps those that are
unused.  For all I know maybe some pass does this already.

Ciao, Duncan.

>
> Index: include/llvm/Support/TargetFolder.h
> ===================================================================
> --- include/llvm/Support/TargetFolder.h (revision 126080)
> +++ include/llvm/Support/TargetFolder.h (working copy)
> @@ -34,8 +34,12 @@
>     /// Fold - Fold the constant using target specific information.
>     Constant *Fold(Constant *C) const {
>       if (ConstantExpr *CE = dyn_cast<ConstantExpr>(C))
> -      if (Constant *CF = ConstantFoldConstantExpression(CE, TD))
> +      if (Constant *CF = ConstantFoldConstantExpression(CE, TD)) {
> +        if (CF != CE)
> +         if (CE->use_empty())
> +           CE->destroyConstant();
>           return CF;
> +     }
>       return C;
>     }
>
>
> ________________________________________
> From: Duncan Sands [baldrick at free.fr]
> Sent: Tuesday, February 22, 2011 5:57 PM
> To: Jin Gu Kang
> Subject: Re: [llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr
>
>> Argument of Fold() is as follows:
>> file: include/llvm/Support/TargetFolder.h on llvm-2.8
>>     Constant *CreateInBoundsGetElementPtr(Constant *C, Value* const *IdxList,
>>                                           unsigned NumIdx) const {
>>       return Fold(ConstantExpr::getInBoundsGetElementPtr(C, IdxList, NumIdx));
>>     }
>>
>> Newly made ConstantExpr is argument of Fold() function.
>
> It may not really be new, that's the point.
>
> Ciao, Duncan.
>
>> -->   ConstantExpr::getInBoundsGetElementPtr(C, IdxList, NumIdx)
>> so if Fold() function make new folded ConstantExpr, I think
>> previous created ConstantExpr is redundant.
>>
>> ________________________________________
>> From: llvm-commits-bounces at cs.uiuc.edu [llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Duncan Sands [baldrick at free.fr]
>> Sent: Tuesday, February 22, 2011 5:35 PM
>> To: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr
>>
>> Hi Jin Gu Kang,
>>
>>> --- include/llvm/Support/TargetFolder.h (revision 126080)
>>> +++ include/llvm/Support/TargetFolder.h (working copy)
>>> @@ -34,8 +34,11 @@
>>> /// Fold - Fold the constant using target specific information.
>>> Constant *Fold(Constant *C) const {
>>> if (ConstantExpr *CE = dyn_cast<ConstantExpr>(C))
>>> - if (Constant *CF = ConstantFoldConstantExpression(CE, TD))
>>> + if (Constant *CF = ConstantFoldConstantExpression(CE, TD)) {
>>> + if (CF != CE)
>>> + CE->destroyConstant();
>>
>> this is wrong.  Constants are global and uniqued: if someone creates a constant,
>> eg: 27, any later attempt to create an identical constant (27) returns the same
>> constant as was created earlier, not a new one.  This means that you can check
>> if constants are identical by comparing their pointer values.  It also means
>> that you can't free a constant like your patch does without first proving that
>> it is not being used anywhere.
>>
>> Ciao, Duncan.
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits