r291123 - CodeGen: plumb header search down to the IAS
Saleem Abdulrasool via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 7 13:27:20 PST 2017
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.
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 <echristo at gmail.com> wrote:
>
>
> On Fri, Jan 6, 2017 at 5:56 AM Hal Finkel <hfinkel at 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 <compnerd at 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 <echristo at 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 at 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/
>> BackendUtil.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/
>> CodeGenAction.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(
>> HeaderSearchOpts),
>> + 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/
>> ObjectFilePCHContainerOperations.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().
>> getDataLayout(),
>> + 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>>
>>
>> --
>> Saleem Abdulrasool
>> compnerd (at) compnerd (dot) org
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing listcfe-commits at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170107/ae218498/attachment-0001.html>
More information about the cfe-commits
mailing list