MaskRay added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4858 + if (Arg *A = Args.getLastArg(options::OPT_fbasicblock_sections_EQ)) { + CmdArgs.push_back( ---------------- If we want to pass the option verbatim, `A->render(Args, CmdArgs);` However, we actually want (a convention) to catch the error at the driver level. So the value checking in Frontend/CompilerInvocation.cpp should be moved here. (Note that you used `err_drv_invalid_value` in Frontend/CompilerInvocation.cpp . drv is short for driver) ================ Comment at: clang/test/CodeGen/basicblock-sections.c:35 +// +// BB_WORLD: .section .text.world,"ax",@progbits +// BB_WORLD: world ---------------- I haven't read through the previous threads whether we should use a .c -> .s test here. Assume we've decided to do that, `@progbits` should be followed by `{{$}}` to ensure there is no unique assembly linkage. ================ Comment at: clang/test/CodeGen/basicblock-sections.c:38 +// BB_WORLD: .section .text.world,"ax",@progbits,unique +// BB_WORLD: a.BB.world +// BB_WORLD: .section .text.another,"ax",@progbits ---------------- If `world` is followed by a colon, the colon should be added. The little detail will make it clear that this is a label. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits