[PATCH] D108872: [IR] Refactor GlobalIFunc to inherit from GlobalObject, Remove GlobalIndirectSymbol

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

Just came across this; still thinking about it.

A couple of thoughts, assuming the direction gains consensus:

- It seems like many of the changes in the patch to use GlobalAlias and/or GlobalIFunc directly instead of GlobalIndirectSymbol could be committed separately / ahead of the change to the IR class hierarchy. (Maybe not worth splitting out until direction has consensus, but would be great to do at that point, to isolate the change as much as possible.)

- Would there be a benefit in leaving behind a GlobalIndirectSymbol utility, modeling a union between GlobalAlias and GlobalIFunc? (I seem to remember a CallSite utility before CallInst and InvokeInst shared the common base class CallBase... but I'm not sure if it'd be similarly useful here.)



================
Comment at: llvm/unittests/IR/ConstantsTest.cpp:388-389
 
-  auto *Global = new GlobalVariable(*M, PtrTy, false,
+  auto *Global = new GlobalVariable(*M, IntTy, false,
                                     GlobalValue::ExternalLinkage, nullptr);
   auto *Alias = GlobalAlias::create(IntTy, 0, GlobalValue::ExternalLinkage,
----------------
It's not obvious how this change is related to the patch... did it slip in accidentally?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108872/new/

https://reviews.llvm.org/D108872