[llvm] r262330 - [LTO] Fix error reporting from lto_module_create_in_local_context()

Petr Pavlu via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 2 01:01:48 PST 2016


David Blaikie beat me in fixing it in r262390.

Thanks!
Petr

On 01/03/16 19:29, Evgenii Stepanov wrote:
> Good point!
> There are two errors, not one :)
> The one I'm complaining about is
>
> LTOModule.cpp:154:10: error: moving a local object in a return
> statement prevents copy elision [-Werror,-Wpessimizing-move]
>    return std::move(Ret);
>
>
> On Tue, Mar 1, 2016 at 11:26 AM, Rafael EspĂ­ndola
> <rafael.espindola at gmail.com> wrote:
>> Are you sure you got the right commit? The error on the bot is:
>>
>> AMDGPUDisassembler.cpp:69:1: error: unused function 'DecodeSGPR_32RegisterClass'
>>
>> Cheers,
>> Rafael
>>
>>
>> On 1 March 2016 at 14:17, Evgenii Stepanov via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> Hi,
>>>
>>> this broke the -Werror build.
>>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/22891/steps/bootstrap%20clang/logs/stdio
>>>
>>> On Tue, Mar 1, 2016 at 5:13 AM, Petr Pavlu via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>> Author: petr.pavlu
>>>> Date: Tue Mar  1 07:13:49 2016
>>>> New Revision: 262330
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=262330&view=rev
>>>> Log:
>>>> [LTO] Fix error reporting from lto_module_create_in_local_context()
>>>>
>>>> Function lto_module_create_in_local_context() would previously
>>>> rely on the default LLVMContext being created for it by
>>>> LTOModule::makeLTOModule(). This context exits the program on
>>>> error and is not arranged to update sLastStringError in
>>>> tools/lto/lto.cpp.
>>>>
>>>> Function lto_module_create_in_local_context() now creates an
>>>> LLVMContext by itself, sets it up correctly to its needs and then
>>>> passes it to LTOModule::createInLocalContext() which takes
>>>> ownership of the context and keeps it present for the lifetime of
>>>> the returned LTOModule.
>>>>
>>>> Function LTOModule::makeLTOModule() is modified to take a
>>>> reference to LLVMContext (instead of a pointer) and no longer
>>>> creates a default context when nullptr is passed to it. Method
>>>> LTOModule::createInContext() that takes a pointer to LLVMContext
>>>> is removed because it allows to pass a nullptr to it. Instead
>>>> LTOModule::createFromBuffer() (that takes a reference to
>>>> LLVMContext) should be used.
>>>>
>>>> Differential Revision: http://reviews.llvm.org/D17715
>>>>
>>>> Added:
>>>>      llvm/trunk/test/tools/llvm-lto/Inputs/empty.bc
>>>> Modified:
>>>>      llvm/trunk/include/llvm/LTO/LTOModule.h
>>>>      llvm/trunk/lib/LTO/LTOModule.cpp
>>>>      llvm/trunk/test/tools/llvm-lto/error.ll
>>>>      llvm/trunk/tools/llvm-lto/llvm-lto.cpp
>>>>      llvm/trunk/tools/lto/lto.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/LTO/LTOModule.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/LTO/LTOModule.h?rev=262330&r1=262329&r2=262330&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/LTO/LTOModule.h (original)
>>>> +++ llvm/trunk/include/llvm/LTO/LTOModule.h Tue Mar  1 07:13:49 2016
>>>> @@ -57,8 +57,6 @@ private:
>>>>     std::vector<const char*>                _asm_undefines;
>>>>
>>>>     LTOModule(std::unique_ptr<object::IRObjectFile> Obj, TargetMachine *TM);
>>>> -  LTOModule(std::unique_ptr<object::IRObjectFile> Obj, TargetMachine *TM,
>>>> -            std::unique_ptr<LLVMContext> Context);
>>>>
>>>>   public:
>>>>     ~LTOModule();
>>>> @@ -100,13 +98,9 @@ public:
>>>>     static ErrorOr<std::unique_ptr<LTOModule>>
>>>>     createFromBuffer(LLVMContext &Context, const void *mem, size_t length,
>>>>                      TargetOptions options, StringRef path = "");
>>>> -
>>>> -  static ErrorOr<std::unique_ptr<LTOModule>>
>>>> -  createInLocalContext(const void *mem, size_t length, TargetOptions options,
>>>> -                       StringRef path);
>>>>     static ErrorOr<std::unique_ptr<LTOModule>>
>>>> -  createInContext(const void *mem, size_t length, TargetOptions options,
>>>> -                  StringRef path, LLVMContext *Context);
>>>> +  createInLocalContext(std::unique_ptr<LLVMContext> Context, const void *mem,
>>>> +                       size_t length, TargetOptions options, StringRef path);
>>>>
>>>>     const Module &getModule() const {
>>>>       return const_cast<LTOModule*>(this)->getModule();
>>>> @@ -206,7 +200,7 @@ private:
>>>>     /// Create an LTOModule (private version).
>>>>     static ErrorOr<std::unique_ptr<LTOModule>>
>>>>     makeLTOModule(MemoryBufferRef Buffer, TargetOptions options,
>>>> -                LLVMContext *Context);
>>>> +                LLVMContext &Context, bool ShouldBeLazy);
>>>>   };
>>>>   }
>>>>   #endif
>>>>
>>>> Modified: llvm/trunk/lib/LTO/LTOModule.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOModule.cpp?rev=262330&r1=262329&r2=262330&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/LTO/LTOModule.cpp (original)
>>>> +++ llvm/trunk/lib/LTO/LTOModule.cpp Tue Mar  1 07:13:49 2016
>>>> @@ -54,11 +54,6 @@ LTOModule::LTOModule(std::unique_ptr<obj
>>>>                        llvm::TargetMachine *TM)
>>>>       : IRFile(std::move(Obj)), _target(TM) {}
>>>>
>>>> -LTOModule::LTOModule(std::unique_ptr<object::IRObjectFile> Obj,
>>>> -                     llvm::TargetMachine *TM,
>>>> -                     std::unique_ptr<LLVMContext> Context)
>>>> -    : OwnedContext(std::move(Context)), IRFile(std::move(Obj)), _target(TM) {}
>>>> -
>>>>   LTOModule::~LTOModule() {}
>>>>
>>>>   /// isBitcodeFile - Returns 'true' if the file (or memory contents) is LLVM
>>>> @@ -110,7 +105,8 @@ LTOModule::createFromFile(LLVMContext &C
>>>>       return EC;
>>>>     }
>>>>     std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrErr.get());
>>>> -  return makeLTOModule(Buffer->getMemBufferRef(), options, &Context);
>>>> +  return makeLTOModule(Buffer->getMemBufferRef(), options, Context,
>>>> +                       /* ShouldBeLazy*/ false);
>>>>   }
>>>>
>>>>   ErrorOr<std::unique_ptr<LTOModule>>
>>>> @@ -130,29 +126,32 @@ LTOModule::createFromOpenFileSlice(LLVMC
>>>>       return EC;
>>>>     }
>>>>     std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrErr.get());
>>>> -  return makeLTOModule(Buffer->getMemBufferRef(), options, &Context);
>>>> +  return makeLTOModule(Buffer->getMemBufferRef(), options, Context,
>>>> +                       /* ShouldBeLazy */ false);
>>>>   }
>>>>
>>>>   ErrorOr<std::unique_ptr<LTOModule>>
>>>>   LTOModule::createFromBuffer(LLVMContext &Context, const void *mem,
>>>>                               size_t length, TargetOptions options,
>>>>                               StringRef path) {
>>>> -  return createInContext(mem, length, options, path, &Context);
>>>> +  StringRef Data((const char *)mem, length);
>>>> +  MemoryBufferRef Buffer(Data, path);
>>>> +  return makeLTOModule(Buffer, options, Context, /* ShouldBeLazy */ false);
>>>>   }
>>>>
>>>>   ErrorOr<std::unique_ptr<LTOModule>>
>>>> -LTOModule::createInLocalContext(const void *mem, size_t length,
>>>> +LTOModule::createInLocalContext(std::unique_ptr<LLVMContext> Context,
>>>> +                                const void *mem, size_t length,
>>>>                                   TargetOptions options, StringRef path) {
>>>> -  return createInContext(mem, length, options, path, nullptr);
>>>> -}
>>>> -
>>>> -ErrorOr<std::unique_ptr<LTOModule>>
>>>> -LTOModule::createInContext(const void *mem, size_t length,
>>>> -                           TargetOptions options, StringRef path,
>>>> -                           LLVMContext *Context) {
>>>>     StringRef Data((const char *)mem, length);
>>>>     MemoryBufferRef Buffer(Data, path);
>>>> -  return makeLTOModule(Buffer, options, Context);
>>>> +  // If we own a context, we know this is being used only for symbol extraction,
>>>> +  // not linking.  Be lazy in that case.
>>>> +  ErrorOr<std::unique_ptr<LTOModule>> Ret =
>>>> +      makeLTOModule(Buffer, options, *Context, /* ShouldBeLazy */ true);
>>>> +  if (Ret)
>>>> +    (*Ret)->OwnedContext = std::move(Context);
>>>> +  return std::move(Ret);
>>>>   }
>>>>
>>>>   static ErrorOr<std::unique_ptr<Module>>
>>>> @@ -187,18 +186,9 @@ parseBitcodeFileImpl(MemoryBufferRef Buf
>>>>
>>>>   ErrorOr<std::unique_ptr<LTOModule>>
>>>>   LTOModule::makeLTOModule(MemoryBufferRef Buffer, TargetOptions options,
>>>> -                         LLVMContext *Context) {
>>>> -  std::unique_ptr<LLVMContext> OwnedContext;
>>>> -  if (!Context) {
>>>> -    OwnedContext = llvm::make_unique<LLVMContext>();
>>>> -    Context = OwnedContext.get();
>>>> -  }
>>>> -
>>>> -  // If we own a context, we know this is being used only for symbol
>>>> -  // extraction, not linking.  Be lazy in that case.
>>>> +                         LLVMContext &Context, bool ShouldBeLazy) {
>>>>     ErrorOr<std::unique_ptr<Module>> MOrErr =
>>>> -      parseBitcodeFileImpl(Buffer, *Context,
>>>> -                           /* ShouldBeLazy */ static_cast<bool>(OwnedContext));
>>>> +      parseBitcodeFileImpl(Buffer, Context, ShouldBeLazy);
>>>>     if (std::error_code EC = MOrErr.getError())
>>>>       return EC;
>>>>     std::unique_ptr<Module> &M = *MOrErr;
>>>> @@ -236,12 +226,7 @@ LTOModule::makeLTOModule(MemoryBufferRef
>>>>     std::unique_ptr<object::IRObjectFile> IRObj(
>>>>         new object::IRObjectFile(Buffer, std::move(M)));
>>>>
>>>> -  std::unique_ptr<LTOModule> Ret;
>>>> -  if (OwnedContext)
>>>> -    Ret.reset(new LTOModule(std::move(IRObj), target, std::move(OwnedContext)));
>>>> -  else
>>>> -    Ret.reset(new LTOModule(std::move(IRObj), target));
>>>> -
>>>> +  std::unique_ptr<LTOModule> Ret(new LTOModule(std::move(IRObj), target));
>>>>     Ret->parseSymbols();
>>>>     Ret->parseMetadata();
>>>>
>>>>
>>>> Added: llvm/trunk/test/tools/llvm-lto/Inputs/empty.bc
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-lto/Inputs/empty.bc?rev=262330&view=auto
>>>> ==============================================================================
>>>>      (empty)
>>>>
>>>> Modified: llvm/trunk/test/tools/llvm-lto/error.ll
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-lto/error.ll?rev=262330&r1=262329&r2=262330&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/test/tools/llvm-lto/error.ll (original)
>>>> +++ llvm/trunk/test/tools/llvm-lto/error.ll Tue Mar  1 07:13:49 2016
>>>> @@ -1,2 +1,5 @@
>>>>   ; RUN: not llvm-lto foobar 2>&1 | FileCheck %s
>>>>   ; CHECK: llvm-lto: error loading file 'foobar': {{N|n}}o such file or directory
>>>> +
>>>> +; RUN: not llvm-lto --list-symbols-only %S/Inputs/empty.bc 2>&1 | FileCheck %s --check-prefix=CHECK-LIST
>>>> +; CHECK-LIST: llvm-lto: error loading file '{{.*}}/Inputs/empty.bc': The file was not recognized as a valid object file
>>>>
>>>> Modified: llvm/trunk/tools/llvm-lto/llvm-lto.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-lto/llvm-lto.cpp?rev=262330&r1=262329&r2=262330&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/llvm-lto/llvm-lto.cpp (original)
>>>> +++ llvm/trunk/tools/llvm-lto/llvm-lto.cpp Tue Mar  1 07:13:49 2016
>>>> @@ -158,8 +158,8 @@ static void diagnosticHandler(const Diag
>>>>       exit(1);
>>>>   }
>>>>
>>>> -static void diagnosticHandlerWithContenxt(const DiagnosticInfo &DI,
>>>> -                                          void *Context) {
>>>> +static void diagnosticHandlerWithContext(const DiagnosticInfo &DI,
>>>> +                                         void *Context) {
>>>>     diagnosticHandler(DI);
>>>>   }
>>>>
>>>> @@ -186,8 +186,11 @@ getLocalLTOModule(StringRef Path, std::u
>>>>     error(BufferOrErr, "error loading file '" + Path + "'");
>>>>     Buffer = std::move(BufferOrErr.get());
>>>>     CurrentActivity = ("loading file '" + Path + "'").str();
>>>> +  std::unique_ptr<LLVMContext> Context = llvm::make_unique<LLVMContext>();
>>>> +  Context->setDiagnosticHandler(diagnosticHandlerWithContext, nullptr, true);
>>>>     ErrorOr<std::unique_ptr<LTOModule>> Ret = LTOModule::createInLocalContext(
>>>> -      Buffer->getBufferStart(), Buffer->getBufferSize(), Options, Path);
>>>> +      std::move(Context), Buffer->getBufferStart(), Buffer->getBufferSize(),
>>>> +      Options, Path);
>>>>     CurrentActivity = "";
>>>>     return std::move(*Ret);
>>>>   }
>>>> @@ -271,7 +274,7 @@ int main(int argc, char **argv) {
>>>>     unsigned BaseArg = 0;
>>>>
>>>>     LLVMContext Context;
>>>> -  Context.setDiagnosticHandler(diagnosticHandlerWithContenxt, nullptr, true);
>>>> +  Context.setDiagnosticHandler(diagnosticHandlerWithContext, nullptr, true);
>>>>
>>>>     LTOCodeGenerator CodeGen(Context);
>>>>
>>>>
>>>> Modified: llvm/trunk/tools/lto/lto.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lto/lto.cpp?rev=262330&r1=262329&r2=262330&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/lto/lto.cpp (original)
>>>> +++ llvm/trunk/tools/lto/lto.cpp Tue Mar  1 07:13:49 2016
>>>> @@ -248,8 +248,14 @@ lto_module_t lto_module_create_in_local_
>>>>                                                   const char *path) {
>>>>     lto_initialize();
>>>>     llvm::TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
>>>> +
>>>> +  // Create a local context. Ownership will be transfered to LTOModule.
>>>> +  std::unique_ptr<LLVMContext> Context = llvm::make_unique<LLVMContext>();
>>>> +  Context->setDiagnosticHandler(diagnosticHandler, nullptr, true);
>>>> +
>>>>     ErrorOr<std::unique_ptr<LTOModule>> M =
>>>> -      LTOModule::createInLocalContext(mem, length, Options, path);
>>>> +      LTOModule::createInLocalContext(std::move(Context), mem, length, Options,
>>>> +                                      path);
>>>>     if (!M)
>>>>       return nullptr;
>>>>     return wrap(M->release());
>>>> @@ -261,8 +267,8 @@ lto_module_t lto_module_create_in_codege
>>>>                                                     lto_code_gen_t cg) {
>>>>     lto_initialize();
>>>>     llvm::TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
>>>> -  ErrorOr<std::unique_ptr<LTOModule>> M = LTOModule::createInContext(
>>>> -      mem, length, Options, path, &unwrap(cg)->getContext());
>>>> +  ErrorOr<std::unique_ptr<LTOModule>> M = LTOModule::createFromBuffer(
>>>> +      unwrap(cg)->getContext(), mem, length, Options, path);
>>>>     return wrap(M->release());
>>>>   }
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



More information about the llvm-commits mailing list