[PATCH] D71144: [AIX] Use csect reference for function address constants
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 14:04:41 PST 2019
hubert.reinterpretcast added a comment.
Just one minor issue with the patch itself, and otherwise, I don't see an issue with the patch in isolation.
I am, however, concerned about the testing. We are adding code to create a `MCSectionXCOFF` with various properties in this patch, and we don't seem to have the corresponding testing here. For example, how would the `C_HIDDEN` manifest itself if not corrected by code inspection? It is also difficult to be confident that a test would actually exercise this instance of the code that sets the csect symbol properties (as opposed to another instance). @DiggerLin, @jasonliu: It would be appreciated if the team can discuss the plan on handing this concern and the timeline on which we are expecting it to close. I would prefer not to have a "long tail" after this patch lands.
================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1767
+ ? TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(F)
+ : XCOFF::C_HIDDEN;
+ MCSectionXCOFF *Csect = OutStreamer->getContext().getXCOFFSection(
----------------
This should be `C_HIDEXT`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71144/new/
https://reviews.llvm.org/D71144
More information about the llvm-commits
mailing list