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

[Jin Gu Kang]

OK. I see.

I found wrong use count of value caused by useless ConstantExpr,
so reported this issue. :)

Thanks your comment.
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