[PATCH] D18634: Don't IPO over functions that can be de-refined

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 30 17:42:33 PDT 2016


> On 2016-Mar-30, at 17:39, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> sanjoy added inline comments.
> 
> ================
> Comment at: include/llvm/IR/GlobalValue.h:300
> @@ -263,1 +299,3 @@
> +  }
> +
>   bool hasExternalLinkage() const { return isExternalLinkage(getLinkage()); }
> ----------------
> joker.eph wrote:
>> Isn't it what `isStrongDefinitionForLinker()` would return?
>> 
>> I don't see why we need a new predicate? (there are probably already too many...)
>> Isn't it what isStrongDefinitionForLinker() would return?
> 
> Yes, except that `isStrongDefinitionForLinker` returns `false` for
> declarations as well.  But most places where I check `mayBeDerefined`
> I also check for `isDeclaration`, so that's not really a problem.
> 
>> I don't see why we need a new predicate? (there are probably already too many...)
> 
> I was trying to have a distinction between the linkage type, and the
> IPO restriction it implies.  So, for instance, if we later add a new
> function attribute that allows for the same replacement semantics (or
> extract out the optimization restrictions into a function attribute,
> and have the linkage type be solely about the linker), then we'd know
> what places to update.
> 
> 
> Having said that, I don't have a strong opinion one way or the other,
> so if the consensus is that it is better to use
> `isStrongDefinitionForLinker`, then I'll change the patch to do that.

Personally I don't mind having both names as long as it's clear that
they're related if you check the header (i.e., implement one in terms
of the other).  The handy place for the massive comment seems useful.
However, I also don't feel strongly about it.

> http://reviews.llvm.org/D18634
> 





More information about the llvm-commits mailing list