tmsriram marked 2 inline comments as done. tmsriram added a comment. In D73307#1972388 <https://reviews.llvm.org/D73307#1972388>, @rnk wrote:
> Regarding the alias attribute, it occurs to me that reimplementing this as an > early LLVM pass would not have that problem. Do you think that would be worth > doing? I can try doing this. If my understanding is right, you are suggesting that doing this later would mean the alias would have already been applied?. However, for multi-versioning symbols, I must parse the target name to insert the module name in between right? ================ Comment at: clang/docs/UsersManual.rst:1684 + linkage symbols. The unique name is obtained by appending the hash of the + full module name to the original symbol. This option is particularly useful + in attributing profile information to the correct function when multiple ---------------- rnk wrote: > tmsriram wrote: > > hubert.reinterpretcast wrote: > > > MaskRay wrote: > > > > rnk wrote: > > > > > I think it's important to be specific here. I'd go so far as to say: > > > > > "When this option is set, compiler hashes the main source file path > > > > > from the command line and appends it to all internal symbols. If a > > > > > program contains multiple objects compiled from the same source file > > > > > path, the symbols are not guaranteed to be unique." > > > > Should option like -ffile-prefix-map= affect the hashed source file > > > > path? I suspect it already does this but I haven't verified. > > > > > > > > This may need tests for Windows and non-Windows, similar to D77223. > > > With respect to @rnk's suggestion, "from the same source file path" > > > implies the same file. I would suggest saying "with the same command-line > > > source file specification". > > I have handled this now and added a test. I saw the reference but don't > > quite understand the need for windows test? I am not familiar with this. > Would the file prefix map not already have been applied when creating the > LLVM IR module source file path? I think it should be, so I'm not sure the > new code you added is in the right place. Module.getSourceFileName() never changes. I checked here and also at AsmPrinter.cpp. I am not sure this a bug, maybe I should just fix that first. ================ Comment at: clang/lib/CodeGen/CodeGenModule.h:31 #include "clang/Basic/XRayLists.h" +#include "clang/Lex/PreprocessorOptions.h" #include "llvm/ADT/DenseMap.h" ---------------- rnk wrote: > Why add this include here? I don't see any reason it is needed. Maybe the cpp > file needs it? Yes, the cpp file needs this to access the MacroPrefixMap. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
