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

[Jin Gu Kang]

I thought useless ConstantExpr makes useless Use information for its constant operands and
they causes inefficient behavior when we use functions like replaceAllUsesWith()
because of wrong Use inforamtion.

so I thought redundant ConstantExpr has to be removed immediately.

--- 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();
         return CF;
+     }
     return C;
   }

Thanks,
Jin-Gu Kang
________________________________________
From: Duncan Sands [baldrick at free.fr]
Sent: Tuesday, February 22, 2011 8:21 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,

> 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.

first note this only impacts constants and not instructions.  Passes which work
on constants need to be aware of the possibility that constants may have uses
that are themselves unused.  See SafeToDestroyConstant in GlobalOpt.cpp for an
example of how one pass deals with this issue.

> 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?

It is not wrong, it is correct because the constant indeed has that many uses.

Ciao, Duncan.

>
> 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