On Sat, Jan 7, 2017 at 1:27 PM, Saleem Abdulrasool via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> I agree with Eric, I don't think that serializing the structure to > std::cout is the right way to handle this. > > What were you thinking of mocking? Wouldn't you really end up mocking the > entire backend? > > That is the small wrinkle with the way that we deal with inline assembly. > It really doesn't mesh too well with LTO if you do something like > `.include`. What happens if you relocated the bitcode between the > compile/link phase? The header search path may be entirely inaccessible at > this point. Or they may be relocated, and the paths may need adjustment. > Another bad case: two different TU's that are linked into the same LTO invocation might have conflicting paths. I.e. one says to search /path/to/foo/ first but the other says to search /path/to/bar/ first. When both paths could satisfy the include, who wins? I don't think we can provide any coherent expectations for users about what will happen. -- Sean Silva > But, I agree with you (Eric) here as well, I find the idea of > serializing the header search options into the module distasteful. > > On Fri, Jan 6, 2017 at 6:22 PM, Eric Christopher <echri...@gmail.com> > wrote: > >> >> >> On Fri, Jan 6, 2017 at 5:56 AM Hal Finkel <hfin...@anl.gov> wrote: >> >>> >>> On 01/05/2017 08:30 PM, Eric Christopher via cfe-commits wrote: >>> >>> Ok, thanks. I agree that it's a problem. I'm definitely open for testing >>> ideas here. There are a few other things in the >>> TargetOptions/MCTargetOptions area that are already problematic to test. >>> >>> >>> I think that we need to add serialization for these structures, and a >>> printing option for them, so that we can test these kinds of things. That >>> having been said, a lot of these things need to end up in attributes so >>> that they work correctly with LTO. Is this one of them? >>> >>> >> We definitely need to figure out testing for this, but I don't know that >> serializing them to std::out is right. Might want to come up with either a >> gtest or gmock way? >> >> That said, ultimately anything that involves parsing at code generation >> time could involve putting it into a module - that said, I really disagree >> with all of the include paths etc being serialized into the module. >> >> -eric >> >> >> >>> -Hal >>> >>> >>> >>> -eric >>> >>> On Thu, Jan 5, 2017 at 6:27 PM Saleem Abdulrasool <compn...@compnerd.org> >>> wrote: >>> >>> This was certainly the problem that I had. The test really needs a way >>> to check that the field was set. As you state, this is a problematic >>> area. The backend already has a test to ensure that the paths are honored, >>> but, I didn't see any way to actually ensure that it was getting sent to >>> the backend otherwise. >>> >>> The module itself doesnt encode the search path, nor is the information >>> in the command line. I can see the argument that the test itself doesn't >>> add much value especially with the backend side testing that the processing >>> of the inclusion does occur correctly. I'll go ahead and remove the test >>> (which already has ended up being a pain to test). >>> >>> On Thu, Jan 5, 2017 at 6:11 PM, Eric Christopher <echri...@gmail.com> >>> wrote: >>> >>> Hi Saleem, >>> >>> Love that you wanted to add a test for it, but I'd really prefer that >>> you not engage the backend here in order to do it. You can verify some of >>> it from the backend and just that the module is correct via the front end >>> if you'd like. Ensuring the paths are correct is a bit of a sticky problem, >>> but this is an API boundary that we just have problems with. >>> >>> TL;DR: Would you mind splitting this test into front end and back end >>> tests and avoid using the backend in clang's test harness? >>> >>> Thanks! >>> >>> -eric >>> >>> On Thu, Jan 5, 2017 at 8:13 AM Saleem Abdulrasool via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>> Author: compnerd >>> Date: Thu Jan 5 10:02:32 2017 >>> New Revision: 291123 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=291123&view=rev >>> Log: >>> CodeGen: plumb header search down to the IAS >>> >>> inline assembly may use the `.include` directive to include other >>> content into the file. Without the integrated assembler, the `-I` group >>> gets passed to the assembler. Emulate this by collecting the header >>> search paths and passing them to the IAS. >>> >>> Resolves PR24811! >>> >>> Added: >>> cfe/trunk/test/CodeGen/include/ >>> cfe/trunk/test/CodeGen/include/function.x >>> cfe/trunk/test/CodeGen/include/module.x >>> cfe/trunk/test/CodeGen/inline-asm-inclusion.c >>> Modified: >>> cfe/trunk/include/clang/CodeGen/BackendUtil.h >>> cfe/trunk/lib/CodeGen/BackendUtil.cpp >>> cfe/trunk/lib/CodeGen/CodeGenAction.cpp >>> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp >>> >>> Modified: cfe/trunk/include/clang/CodeGen/BackendUtil.h >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>> CodeGen/BackendUtil.h?rev=291123&r1=291122&r2=291123&view=diff >>> ============================================================ >>> ================== >>> --- cfe/trunk/include/clang/CodeGen/BackendUtil.h (original) >>> +++ cfe/trunk/include/clang/CodeGen/BackendUtil.h Thu Jan 5 10:02:32 >>> 2017 >>> @@ -21,6 +21,7 @@ namespace llvm { >>> >>> namespace clang { >>> class DiagnosticsEngine; >>> + class HeaderSearchOptions; >>> class CodeGenOptions; >>> class TargetOptions; >>> class LangOptions; >>> @@ -34,7 +35,8 @@ namespace clang { >>> Backend_EmitObj ///< Emit native object files >>> }; >>> >>> - void EmitBackendOutput(DiagnosticsEngine &Diags, const >>> CodeGenOptions &CGOpts, >>> + void EmitBackendOutput(DiagnosticsEngine &Diags, const >>> HeaderSearchOptions &, >>> + const CodeGenOptions &CGOpts, >>> const TargetOptions &TOpts, const LangOptions >>> &LOpts, >>> const llvm::DataLayout &TDesc, llvm::Module *M, >>> BackendAction Action, >>> >>> Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/Ba >>> ckendUtil.cpp?rev=291123&r1=291122&r2=291123&view=diff >>> ============================================================ >>> ================== >>> --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Jan 5 10:02:32 2017 >>> @@ -14,6 +14,7 @@ >>> #include "clang/Frontend/CodeGenOptions.h" >>> #include "clang/Frontend/FrontendDiagnostic.h" >>> #include "clang/Frontend/Utils.h" >>> +#include "clang/Lex/HeaderSearchOptions.h" >>> #include "llvm/ADT/SmallSet.h" >>> #include "llvm/ADT/StringExtras.h" >>> #include "llvm/ADT/StringSwitch.h" >>> @@ -32,6 +33,7 @@ >>> #include "llvm/IR/ModuleSummaryIndex.h" >>> #include "llvm/IR/Verifier.h" >>> #include "llvm/LTO/LTOBackend.h" >>> +#include "llvm/MC/MCAsmInfo.h" >>> #include "llvm/MC/SubtargetFeature.h" >>> #include "llvm/Object/ModuleSummaryIndexObjectFile.h" >>> #include "llvm/Passes/PassBuilder.h" >>> @@ -61,6 +63,7 @@ namespace { >>> >>> class EmitAssemblyHelper { >>> DiagnosticsEngine &Diags; >>> + const HeaderSearchOptions &HSOpts; >>> const CodeGenOptions &CodeGenOpts; >>> const clang::TargetOptions &TargetOpts; >>> const LangOptions &LangOpts; >>> @@ -100,11 +103,14 @@ private: >>> raw_pwrite_stream &OS); >>> >>> public: >>> - EmitAssemblyHelper(DiagnosticsEngine &_Diags, const CodeGenOptions >>> &CGOpts, >>> + EmitAssemblyHelper(DiagnosticsEngine &_Diags, >>> + const HeaderSearchOptions &HeaderSearchOpts, >>> + const CodeGenOptions &CGOpts, >>> const clang::TargetOptions &TOpts, >>> const LangOptions &LOpts, Module *M) >>> - : Diags(_Diags), CodeGenOpts(CGOpts), TargetOpts(TOpts), >>> LangOpts(LOpts), >>> - TheModule(M), CodeGenerationTime("codegen", "Code Generation >>> Time") {} >>> + : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts), >>> + TargetOpts(TOpts), LangOpts(LOpts), TheModule(M), >>> + CodeGenerationTime("codegen", "Code Generation Time") {} >>> >>> ~EmitAssemblyHelper() { >>> if (CodeGenOpts.DisableFree) >>> @@ -584,12 +590,18 @@ void EmitAssemblyHelper::CreateTargetMac >>> Options.MCOptions.MCNoExecStack = CodeGenOpts.NoExecStack; >>> Options.MCOptions.MCIncrementalLinkerCompatible = >>> CodeGenOpts.IncrementalLinkerCompatible; >>> - Options.MCOptions.MCPIECopyRelocations = >>> - CodeGenOpts.PIECopyRelocations; >>> + Options.MCOptions.MCPIECopyRelocations = >>> CodeGenOpts.PIECopyRelocations; >>> Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings; >>> Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose; >>> Options.MCOptions.PreserveAsmComments = >>> CodeGenOpts.PreserveAsmComments; >>> Options.MCOptions.ABIName = TargetOpts.ABI; >>> + for (const auto &Entry : HSOpts.UserEntries) >>> + if (!Entry.IsFramework && >>> + (Entry.Group == frontend::IncludeDirGroup::Quoted || >>> + Entry.Group == frontend::IncludeDirGroup::Angled || >>> + Entry.Group == frontend::IncludeDirGroup::System)) >>> + Options.MCOptions.IASSearchPaths.push_back( >>> + Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + >>> Entry.Path); >>> >>> TM.reset(TheTarget->createTargetMachine(Triple, TargetOpts.CPU, >>> FeaturesStr, >>> Options, RM, CM, OptLevel)); >>> @@ -929,17 +941,19 @@ static void runThinLTOBackend(const Code >>> } >>> >>> void clang::EmitBackendOutput(DiagnosticsEngine &Diags, >>> + const HeaderSearchOptions &HeaderOpts, >>> const CodeGenOptions &CGOpts, >>> const clang::TargetOptions &TOpts, >>> - const LangOptions &LOpts, const >>> llvm::DataLayout &TDesc, >>> - Module *M, BackendAction Action, >>> + const LangOptions &LOpts, >>> + const llvm::DataLayout &TDesc, Module *M, >>> + BackendAction Action, >>> std::unique_ptr<raw_pwrite_stream> OS) { >>> if (!CGOpts.ThinLTOIndexFile.empty()) { >>> runThinLTOBackend(CGOpts, M, std::move(OS)); >>> return; >>> } >>> >>> - EmitAssemblyHelper AsmHelper(Diags, CGOpts, TOpts, LOpts, M); >>> + EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, >>> M); >>> >>> if (CGOpts.ExperimentalNewPassManager) >>> AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS)); >>> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/Co >>> deGenAction.cpp?rev=291123&r1=291122&r2=291123&view=diff >>> ============================================================ >>> ================== >>> --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Thu Jan 5 10:02:32 2017 >>> @@ -44,6 +44,7 @@ namespace clang { >>> virtual void anchor(); >>> DiagnosticsEngine &Diags; >>> BackendAction Action; >>> + const HeaderSearchOptions &HeaderSearchOpts; >>> const CodeGenOptions &CodeGenOpts; >>> const TargetOptions &TargetOpts; >>> const LangOptions &LangOpts; >>> @@ -77,8 +78,8 @@ namespace clang { >>> const SmallVectorImpl<std::pair<unsigned, llvm::Module *>> >>> &LinkModules, >>> std::unique_ptr<raw_pwrite_stream> OS, LLVMContext &C, >>> CoverageSourceInfo *CoverageInfo = nullptr) >>> - : Diags(Diags), Action(Action), CodeGenOpts(CodeGenOpts), >>> - TargetOpts(TargetOpts), LangOpts(LangOpts), >>> + : Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchO >>> pts), >>> + CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), >>> LangOpts(LangOpts), >>> AsmOutStream(std::move(OS)), Context(nullptr), >>> LLVMIRGeneration("irgen", "LLVM IR Generation Time"), >>> LLVMIRGenerationRefCount(0), >>> @@ -225,8 +226,8 @@ namespace clang { >>> >>> EmbedBitcode(getModule(), CodeGenOpts, llvm::MemoryBufferRef()); >>> >>> - EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts, >>> - C.getTargetInfo().getDataLayout(), >>> + EmitBackendOutput(Diags, HeaderSearchOpts, CodeGenOpts, >>> TargetOpts, >>> + LangOpts, C.getTargetInfo().getDataLayout(), >>> getModule(), Action, std::move(AsmOutStream)); >>> >>> Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext); >>> @@ -898,9 +899,10 @@ void CodeGenAction::ExecuteAction() { >>> Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler, >>> &CI.getDiagnostics()); >>> >>> - EmitBackendOutput(CI.getDiagnostics(), CI.getCodeGenOpts(), >>> TargetOpts, >>> - CI.getLangOpts(), CI.getTarget().getDataLayout(), >>> - TheModule.get(), BA, std::move(OS)); >>> + EmitBackendOutput(CI.getDiagnostics(), CI.getHeaderSearchOpts(), >>> + CI.getCodeGenOpts(), TargetOpts, CI.getLangOpts(), >>> + CI.getTarget().getDataLayout(), TheModule.get(), >>> BA, >>> + std::move(OS)); >>> return; >>> } >>> >>> >>> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/Ob >>> jectFilePCHContainerOperations.cpp?rev=291123&r1=291122&r2= >>> 291123&view=diff >>> ============================================================ >>> ================== >>> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp >>> (original) >>> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Jan >>> 5 10:02:32 2017 >>> @@ -282,7 +282,7 @@ public: >>> // Print the IR for the PCH container to the debug output. >>> llvm::SmallString<0> Buffer; >>> clang::EmitBackendOutput( >>> - Diags, CodeGenOpts, TargetOpts, LangOpts, >>> + Diags, HeaderSearchOpts, CodeGenOpts, TargetOpts, LangOpts, >>> Ctx.getTargetInfo().getDataLayout(), M.get(), >>> BackendAction::Backend_EmitLL, >>> llvm::make_unique<llvm::raw_svector_ostream>(Buffer)); >>> @@ -290,9 +290,10 @@ public: >>> }); >>> >>> // Use the LLVM backend to emit the pch container. >>> - clang::EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts, >>> - Ctx.getTargetInfo().getDataLayout(), >>> M.get(), >>> - BackendAction::Backend_EmitObj, >>> std::move(OS)); >>> + clang::EmitBackendOutput(Diags, HeaderSearchOpts, CodeGenOpts, >>> TargetOpts, >>> + LangOpts, Ctx.getTargetInfo().getDataLay >>> out(), >>> + M.get(), BackendAction::Backend_EmitObj, >>> + std::move(OS)); >>> >>> // Free the memory for the temporary buffer. >>> llvm::SmallVector<char, 0> Empty; >>> >>> Added: cfe/trunk/test/CodeGen/include/function.x >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ >>> include/function.x?rev=291123&view=auto >>> ============================================================ >>> ================== >>> --- cfe/trunk/test/CodeGen/include/function.x (added) >>> +++ cfe/trunk/test/CodeGen/include/function.x Thu Jan 5 10:02:32 2017 >>> @@ -0,0 +1 @@ >>> +FUNCTION = 1 >>> >>> Added: cfe/trunk/test/CodeGen/include/module.x >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ >>> include/module.x?rev=291123&view=auto >>> ============================================================ >>> ================== >>> --- cfe/trunk/test/CodeGen/include/module.x (added) >>> +++ cfe/trunk/test/CodeGen/include/module.x Thu Jan 5 10:02:32 2017 >>> @@ -0,0 +1 @@ >>> +MODULE = 1 >>> >>> Added: cfe/trunk/test/CodeGen/inline-asm-inclusion.c >>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ >>> inline-asm-inclusion.c?rev=291123&view=auto >>> ============================================================ >>> ================== >>> --- cfe/trunk/test/CodeGen/inline-asm-inclusion.c (added) >>> +++ cfe/trunk/test/CodeGen/inline-asm-inclusion.c Thu Jan 5 10:02:32 >>> 2017 >>> @@ -0,0 +1,10 @@ >>> +// RUN: %clang_cc1 -I %p/include -S -o - %s | FileCheck %s >>> + >>> +__asm__(".include \"module.x\""); >>> +void function(void) { >>> + __asm__(".include \"function.x\""); >>> +} >>> + >>> +// CHECK: MODULE = 1 >>> +// CHECK: FUNCTION = 1 >>> + >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>> >>> >>> >>> -- >>> Saleem Abdulrasool >>> compnerd (at) compnerd (dot) org >>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing >>> listcfe-comm...@lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>> >>> -- >>> Hal Finkel >>> Lead, Compiler Technology and Programming Languages >>> Leadership Computing Facility >>> Argonne National Laboratory >>> >>> > > > -- > Saleem Abdulrasool > compnerd (at) compnerd (dot) org > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
