subject:"\[PATCH\] D90366\: \[ThinLTO\] Strenghten the test for .llvmcmd embedding"

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

2020-10-29 Thread Mircea Trofin via Phabricator via cfe-commits

mtrofin updated this revision to Diff 301641.
mtrofin added a comment.

fixed sentence


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90366/new/

https://reviews.llvm.org/D90366

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/thinlto_embed_bitcode.ll


Index: clang/test/CodeGen/thinlto_embed_bitcode.ll
===
--- clang/test/CodeGen/thinlto_embed_bitcode.ll
+++ clang/test/CodeGen/thinlto_embed_bitcode.ll
@@ -17,12 +17,20 @@
 ; round-trip through compilation and ensure the objects are the same.
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c 
-fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s 
--check-prefixes=CHECK-ELF,CHECK-ELF-CMD
+; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
+; RUN: grep x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 
 ; the exact same .o as we originally did.
 ; RUN: rm %t1.bc %t2.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir 
%t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm 
-lto-embed-bitcode=post-merge-pre-opt -mllvm -thinlto-assume-merged
+;
+; The resulting .o is almost the same, but the .llvmcmd section would be
+; different because of the extra -thinlto-assume-merged.
+; A simple, meaningful comparison is to just compare the stripped objects.
+; RUN: llvm-strip --strip-all %t-redo.o 
+; RUN: llvm-strip --strip-all %t.o 
 ; RUN: diff %t-redo.o %t.o
 
 ; CHECK-ELF:  .text   PROGBITS  [[#%x,OFF:]] [[#%x,SIZE:]] 
00 AX 0
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1095,23 +1095,21 @@
   // FIXME: For backend options that are not yet recorded as function
   // attributes in the IR, keep track of them so we can embed them in a
   // separate data section and use them when building the bitcode.
-  if (Opts.getEmbedBitcode() == CodeGenOptions::Embed_All) {
-for (const auto  : Args) {
-  // Do not encode output and input.
-  if (A->getOption().getID() == options::OPT_o ||
-  A->getOption().getID() == options::OPT_INPUT ||
-  A->getOption().getID() == options::OPT_x ||
-  A->getOption().getID() == options::OPT_fembed_bitcode ||
-  A->getOption().matches(options::OPT_W_Group))
-continue;
-  ArgStringList ASL;
-  A->render(Args, ASL);
-  for (const auto  : ASL) {
-StringRef ArgStr(arg);
-Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
-// using \00 to separate each commandline options.
-Opts.CmdArgs.push_back('\0');
-  }
+  for (const auto  : Args) {
+// Do not encode output and input.
+if (A->getOption().getID() == options::OPT_o ||
+A->getOption().getID() == options::OPT_INPUT ||
+A->getOption().getID() == options::OPT_x ||
+A->getOption().getID() == options::OPT_fembed_bitcode ||
+A->getOption().matches(options::OPT_W_Group))
+  continue;
+ArgStringList ASL;
+A->render(Args, ASL);
+for (const auto  : ASL) {
+  StringRef ArgStr(arg);
+  Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
+  // using \00 to separate each commandline options.
+  Opts.CmdArgs.push_back('\0');
 }
   }
 


Index: clang/test/CodeGen/thinlto_embed_bitcode.ll
===
--- clang/test/CodeGen/thinlto_embed_bitcode.ll
+++ clang/test/CodeGen/thinlto_embed_bitcode.ll
@@ -17,12 +17,20 @@
 ; round-trip through compilation and ensure the objects are the same.
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s --check-prefixes=CHECK-ELF,CHECK-ELF-CMD
+; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
+; RUN: grep x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 
 ; the exact same .o as we originally did.
 ; RUN: rm %t1.bc %t2.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir %t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt -mllvm -thinlto-assume-merged
+;

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

2020-10-29 Thread Mircea Trofin via Phabricator via cfe-commits

mtrofin added a comment.

In D90366#2362052 , @tejohnson wrote:

> The motivation and effects of the change are unclear to me. Does this mean 
> that the .llvmcmd section is always emitted? Or always emitted whenever any 
> bitcode embedding is requested?

This just means the arguments are captured in memory. Nothing else changes. The 
only penalty is that a char vector (the captured args) are carried around even 
if .llvmcmd wasn't generated.

We could maybe add a test that no explicit optin to emitting .llvmcmd => no 
section (briefly looked, doesn't seem to exist for either non- or thinlto)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90366/new/

https://reviews.llvm.org/D90366

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

2020-10-29 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added a comment.

The motivation and effects of the change are unclear to me. Does this mean that 
the .llvmcmd section is always emitted? Or always emitted whenever any bitcode 
embedding is requested?




Comment at: clang/test/CodeGen/thinlto_embed_bitcode.ll:28
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir 
%t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm 
-lto-embed-bitcode=post-merge-pre-opt -mllvm -thinlto-assume-merged
+; The resulting .o is almost the 
+; RUN: llvm-strip --strip-all %t-redo.o 

Incomplete sentence


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90366/new/

https://reviews.llvm.org/D90366

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

2020-10-28 Thread Mircea Trofin via Phabricator via cfe-commits

mtrofin created this revision.
mtrofin added a reviewer: tejohnson.
Herald added subscribers: cfe-commits, steven_wu, hiraditya, inglorion.
Herald added a reviewer: alexshap.
Herald added a project: clang.
mtrofin requested review of this revision.

Always populate the CodeGenOptions::CmdArgs - the overhead is likely
negligible in the grand schema of things, and it keeps the using code
simple.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90366

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/thinlto_embed_bitcode.ll


Index: clang/test/CodeGen/thinlto_embed_bitcode.ll
===
--- clang/test/CodeGen/thinlto_embed_bitcode.ll
+++ clang/test/CodeGen/thinlto_embed_bitcode.ll
@@ -17,12 +17,17 @@
 ; round-trip through compilation and ensure the objects are the same.
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c 
-fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s 
--check-prefixes=CHECK-ELF,CHECK-ELF-CMD
+; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
+; RUN: grep x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 
 ; the exact same .o as we originally did.
 ; RUN: rm %t1.bc %t2.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir 
%t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm 
-lto-embed-bitcode=post-merge-pre-opt -mllvm -thinlto-assume-merged
+; The resulting .o is almost the 
+; RUN: llvm-strip --strip-all %t-redo.o 
+; RUN: llvm-strip --strip-all %t.o 
 ; RUN: diff %t-redo.o %t.o
 
 ; CHECK-ELF:  .text   PROGBITS  [[#%x,OFF:]] [[#%x,SIZE:]] 
00 AX 0
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1095,23 +1095,21 @@
   // FIXME: For backend options that are not yet recorded as function
   // attributes in the IR, keep track of them so we can embed them in a
   // separate data section and use them when building the bitcode.
-  if (Opts.getEmbedBitcode() == CodeGenOptions::Embed_All) {
-for (const auto  : Args) {
-  // Do not encode output and input.
-  if (A->getOption().getID() == options::OPT_o ||
-  A->getOption().getID() == options::OPT_INPUT ||
-  A->getOption().getID() == options::OPT_x ||
-  A->getOption().getID() == options::OPT_fembed_bitcode ||
-  A->getOption().matches(options::OPT_W_Group))
-continue;
-  ArgStringList ASL;
-  A->render(Args, ASL);
-  for (const auto  : ASL) {
-StringRef ArgStr(arg);
-Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
-// using \00 to separate each commandline options.
-Opts.CmdArgs.push_back('\0');
-  }
+  for (const auto  : Args) {
+// Do not encode output and input.
+if (A->getOption().getID() == options::OPT_o ||
+A->getOption().getID() == options::OPT_INPUT ||
+A->getOption().getID() == options::OPT_x ||
+A->getOption().getID() == options::OPT_fembed_bitcode ||
+A->getOption().matches(options::OPT_W_Group))
+  continue;
+ArgStringList ASL;
+A->render(Args, ASL);
+for (const auto  : ASL) {
+  StringRef ArgStr(arg);
+  Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
+  // using \00 to separate each commandline options.
+  Opts.CmdArgs.push_back('\0');
 }
   }
 


Index: clang/test/CodeGen/thinlto_embed_bitcode.ll
===
--- clang/test/CodeGen/thinlto_embed_bitcode.ll
+++ clang/test/CodeGen/thinlto_embed_bitcode.ll
@@ -17,12 +17,17 @@
 ; round-trip through compilation and ensure the objects are the same.
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s --check-prefixes=CHECK-ELF,CHECK-ELF-CMD
+; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
+; RUN: grep x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 
 ; the exact same .o as we originally did.
 ; RUN: rm %t1.bc %t2.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir %t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

4 matches

Site Navigation

Mail list logo

Footer information