efriedma added inline comments.
================ Comment at: lib/Analysis/CFG.cpp:3137 + + Block = createBlock(false); + Block->setTerminator(G); ---------------- Passing add_successor=false seems suspect; this could probably use more test coverage. ================ Comment at: lib/CodeGen/CGStmt.cpp:2182 + } + } + ---------------- jyu2 wrote: > nickdesaulniers wrote: > > If this new block was moved closer to the new one on L2227, I assume they > > could be combined and possibly `IsGCCAsmGoto` be removed? The code > > currently in between doesn't appear at first site to depend on info from > > this block, though maybe I may be missing it. > The labels need be processed before Clobbers Before the clobbers? I don't see the relationship; the clobbers are just a string. Granted, it does need to be before we compute the type, which is shared code. ================ Comment at: lib/CodeGen/CGStmt.cpp:1884 +template <typename T> +void CodeGenFunction::UpdateCallInst( + T &Result, bool HasSideEffect, bool ReadOnly, bool ReadNone, ---------------- rsmith wrote: > This is not a reasonable function name for a function that is specific to > `AsmStmt`s. Please also make this a (static) non-member function, since it > cannot be called outside this source file. Does this really need to be a template function? I think all the functions you're calling on "Result" are members of CallBase. ================ Comment at: lib/CodeGen/CGStmt.cpp:2188 + std::to_string(NumGccAsmGotoStmts)); + NumGccAsmGotoStmts++; + } ---------------- The NumGccAsmGotoStmts variable is pointless; if names are enabled, createBasicBlock will automatically number all the blocks named "normal". ================ Comment at: test/Parser/asm-goto.c:1 +// RUN: %clang_cc1 %s + ---------------- What is this testing? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
