[PATCH] D18433: [IFUNC] Introduce GlobalIndirectSymbol as a base class for aliases and ifuncs

Duncan Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 31 06:48:59 PDT 2016



> On Mar 31, 2016, at 1:02 AM, Dmitry Polukhin <dmitry.polukhin at gmail.com> wrote:
> 
>> On Wed, Mar 30, 2016 at 8:33 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> > +  /// These methods set and retrieve indirect symbol.
>> > +  void setIndirectSymbol(Constant *Symbol) {
>> > +    setOperand(0, Symbol);
>> > +  }
>> 
>> Should setIndirectSymbol() be protected?  I feel like the IFunc
>> version might want to add some extra checks about the type of
>> symbol passed in, or constrain the type somehow.
>> 
>> Although maybe those checks would fit better in the verifier?
> 
> Both approaches have sense to me. Having setIndirectSymbol public gives an ability to use GlobalIndirectSymbol polymorphic (for example, it is used in BitcodeReader in ifunc patch). But GlobalAlias does have assert in setAliasee for types, everything else is checked in verifier. I would prefer to keep setIndirectSymbol public because setOperand is public so it is possible to skip setAliasee check anyway. But if you think that it should be private, I have no objections. please let me know what do you prefer.

Good point about setOperand.  LGTM as is. 

> 
> Duncan and Eric, thank you for review!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160331/3ae683fe/attachment.html>


More information about the llvm-commits mailing list