[PATCH] D33436: [ARM] Create relocation for Thumb functions calling ARM fns.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

In general that looks good to me. I think it would be best not to hoist the check for STT_FUNC and STT_GNU_IFUNC into MC as this is common to all Targets and STT_GNU_IFUNC is defined in the STT_LOOS (OS specific range) and hence could mean something different on another platform/target.



================
Comment at: lib/MC/MCSymbolELF.cpp:197
 
+bool MCSymbolELF::isFunction() const {
+  return getType() == ELF::STT_FUNC || getType() == ELF::STT_GNU_IFUNC;
----------------
I think that we should be careful in putting this in MCSymbolELF. The STT_FUNC is generic ELF and common to all Targets but STT_GNU_IFUNC is defined in the OS specific range [STT_LOOS, STT_HIOS] so there is an outside chance that it is defined to something else on a non ARM Target.


https://reviews.llvm.org/D33436