[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-29 Thread Mircea Trofin via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGafb4efd3bcc6: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled (authored by aidengrossman, committed by mtrofin). Repository:

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-29 Thread Mircea Trofin via Phabricator via cfe-commits
mtrofin added a comment. In D130620#3686357 , @aidengrossman wrote: > Thanks for the review and your suggestions. Do you mind pushing this commit? > I don't currently have commit access to LLVM. Thanks. I can do it. Repository: rG LLVM Github Monor

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-28 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman added a comment. Thanks for the review and your suggestions. Do you mind pushing this commit? I don't currently have commit access to LLVM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130620/new/ https://reviews.llvm.org/D1

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-28 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land. LGTM, thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130620/new/ https://reviews.llvm.org/D130620

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-28 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman updated this revision to Diff 448477. aidengrossman added a comment. Rebased against main and adjusted unit tests to check directly for the cc1 flag in CmdArgs which is directly consumed by the bitcode writer while creating the .llvmcmd section. Also changed the flags to better match

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-28 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment. Thanks for adding the tests. They are failing in CI though, and I don't think they're entirely correct. My understanding is that the goal is to make sure Clang doesn't drop the `-cc1` flag when embedding command-lines into object files. During normal compilation,

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-27 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman updated this revision to Diff 448152. aidengrossman marked an inline comment as done. aidengrossman added a comment. Added two unit tests to ensure functionality is correct in both the assertion case and the default case. Should be sufficient to ensure correct functionality without u

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-27 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment. The code change looks good to me, could you also include a test case for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130620/new/ https://reviews.llvm.org/D130620 ___

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-27 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman marked an inline comment as done. aidengrossman added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4549 +Invocation.generateCC1CommandLine(Args, SA); +Args.insert(Args.begin(), "-cc1"); + }, jansvoboda1

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-27 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman updated this revision to Diff 448132. aidengrossman added a comment. Moved insertion of cc1 flag to prevent shifting entire array Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130620/new/ https://reviews.llvm.org/D130620 Files: cla

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-27 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment. Thanks for the fix! Left one suggestion. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4549 +Invocation.generateCC1CommandLine(Args, SA); +Args.insert(Args.begin(), "-cc1"); + }, This will shift all gen

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

2022-07-27 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman created this revision. Herald added a project: All. aidengrossman requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently when assertions are enabled, the cc1 flag is not inserted into the llvmcmd section of object files wit