- User Since
- Apr 30 2013, 5:34 PM (387 w, 1 d)
Tue, Sep 29
(you local commit is based of f20918b4b15 apparently)
Arc says " INFO Base commit is not in local repository; trying to fetch." ; seems like you have local changes?
Rebase, test is passing so removing XFAIL
Seems like a subset of https://reviews.llvm.org/D88362 ? Seems like you missed it?
Thanks for the fix!
Mon, Sep 28
I suspect we're getting into "vocabulary" here, but I still call this "assert" even though they aren't technically: I actually write it this way https://github.com/llvm/llvm-project/blob/master/mlir/include/mlir/IR/OpDefinition.h#L1274-L1279
But you mention this is to guard a case in the generated parser, why can't we
insert verification & return there? Parsing returns an error and all, and
so if detected there then we already have error propagation path.
Ultimately the only difference between assert vs report_fatal_error is whether it is checked in production and how much it'll be in the path of the runtime execution and binary size, otherwise neither is recoverable.
Some users of LLVM ship clang in production with asserts enabled, if you care about always having checks enabled, that's likely the way to go.
LG but please wait for @herhut to have a look
Yes this is more defensive than what is usually done in LLVM, for example we don't re-check everywhere the invariant that are supposed to be enforced by the verifier: even though you can invoke a pass on invalid IR.
I have a somewhat growing concern over the inconsistent use of error handling mechanism (assert vs report_fatal_error).
Remove the new option and rely on TENSORFLOW_C_LIB_PATH to be set instead
Sun, Sep 27
Sat, Sep 26
Replace some auto with explicit types
Fri, Sep 25
In general the semantics of this op isn't clear to me. It seems that we "hide" the SIMT aspect by "faking" a scalar computation.
Have you looked into structuring this in a region instead?
Thu, Sep 24
LGTM on the principle, whenever you get it to work :)
I'm perfectly fine if you push such trivial fixes without code review FYI
Wed, Sep 23
Tweak variable name
Tue, Sep 22
It wasn't clear from the description that this is actually fixing something that wasn't working, can you update the description in the commit?
I tried to review, but I'm too unfamiliar with this code at this point and won't have enough time to page-in the existing one and review the new one.
I can't find a discussion and rationale on Discourse, was this discussed?
LLVM does not have intrinsics for atan I believe, it is just a libcall: what is the tradeoff about adding it to standard here?
Mon, Sep 21
This is supposed to be handled in the translate registration themselves: https://github.com/llvm/llvm-project/blob/master/mlir/lib/Dialect/SPIRV/Serialization/TranslateRegistration.cpp#L175
Sun, Sep 20
Sat, Sep 19
Fri, Sep 18