Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Add Orc Error Handling #22611

Closed Quuxplusone closed 5 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR22612
Status RESOLVED FIXED
Importance P normal
Reported by Lang Hames (lhames@gmail.com)
Reported on 2015-02-16 20:28:53 -0800
Last modified on 2019-05-20 14:55:53 -0700
Version trunk
Hardware Other All
CC dblaikie@gmail.com, filcab@gmail.com, inolen@gmail.com, llvm-bugs@lists.llvm.org, massad@gmail.com, rafael@espindo.la, rnk@google.com, stefan.graenitz@gmail.com, vivekvpandya@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
There's currently no way for Orc to report errors to the user.

We sould add a JITErrorLog class and make sure a reference to it is available
in each layer. Straw man implementation:

class JITErrorLog {
public:
  void addError(std::string ErrMsg) { Errors.push_back(std::move(ErrMsg); }
  bool allOk() const { return Errors.empty(); }
  void reset() { Errors.clear(); }
private:
  std::vector<std::string> Errors;
};

More information can be added as required.

Possible gotchas: We want Orc to run multi-threaded day, but we don't want
cross-talk between threads. Syncronisation and extra support will be required
to ensure that errors are reported and responded to on the appropriate thread.
Quuxplusone commented 9 years ago

Don't we have error reporting on the LLVMContext now? Is that appropriate?

Quuxplusone commented 9 years ago
Hrm. I hadn't thought of that.

Off the top of my head I have a couple of (potentially minor) objections:

(1) RuntimeDyld doesn't need/want an LLVMContext. In *most* contexts where
you're using it you'll already have an LLVMContext too, but not everywhere. See
e.g. llvm-rtdyld. Also I'm told people are using it in isolation in the real
world. (That's scary in its own right, but still - apparently a thing...).

(2) I eventually want Orc to support threaded compilation, so there would be
multiple LLVMContexts in play. In that case we'd still want one global error
reporting mechanism.

Have I overlooked anything obvious there? If we *don't* go for LLVMContext I'd
certainly be happy to make sure that whatever error reporting mechanism we do
have can be optionally forwarded to a given context. That will make life easy
in the common case where there's just one LLVMContext.
Quuxplusone commented 9 years ago
(In reply to comment #2)
> Hrm. I hadn't thought of that.
>
> Off the top of my head I have a couple of (potentially minor) objections:
>
> (1) RuntimeDyld doesn't need/want an LLVMContext. In *most* contexts where
> you're using it you'll already have an LLVMContext too, but not everywhere.
> See e.g. llvm-rtdyld. Also I'm told people are using it in isolation in the
> real world. (That's scary in its own right, but still - apparently a
> thing...).

Note that there is some desire to move the diagnostic reporting code out of
lib/IR into lib/Support so that it can be used in places where a LLVMContext is
not easy to get.
Quuxplusone commented 8 years ago

We can use the new Error class (include/Support/Error.h) for this now.

I've started threading Error through the implementation of RuntimeDyld. The next step is to actually tackle the SymbolResolver/MemoryManager/ORC interface redesign.

Quuxplusone commented 5 years ago

Most of the Orc code should be errorized nowadays, right? Are there any particular pieces missing?

Quuxplusone commented 5 years ago

Are there any particular pieces missing?

I don't think so (maybe there is some missing plumbing in ORCv1, but we're deprecating that now anyway). ORCv2 was written from the ground up with llvm::Error and has pretty thorough error plumbing. I'm calling this fixed. :)