Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Mehdi Amini via cfe-commits]

But what pipeline do we setup? For instance with ThinLTO we reduced the amount 
of passes ran during the compile phase with the expectation that more will run 
during the link, this would get fuzzy here...

> On Jun 5, 2016, at 6:49 PM, Davide Italiano  wrote:
> 
> davide added a comment.
> 
> In http://reviews.llvm.org/D21006#449409, @dexonsmith wrote:
> 
>> I agree with Mehdi.  I expect `-S -flto` to give equivalent output to `-c 
>> -flto`.
>> 
>> In effect, with this change, `-flto -S` would silently ignore the `-flto` 
>> flag.  That doesn't make sense to me.
> 
> 
> I guess there's enough disagreement to hold on on this change.
> My $0.02: When I ask for -S I really expect asm to be produced, but maybe 
> it's just me =)
> Lots of build systems have invocations of clang like:
> 
>  CFLAGS= -S
>  ${CC} ${CFLAGS} blah.c -o blah.s
>  sed -e ...
> 
> When CFLAGS is changed, as in:
> 
>  CFLAGS += -flto
> 
> this is generally passed globally. So, not emitting ASM in this case makes 
> LTO non transparent for the build system, i.e. Makefiles need to pass 
> `-fno-lto`, which might not be terrible overall, just not what I expect.
> 
> 
> http://reviews.llvm.org/D21006
> 
> 
> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Mehdi AMINI via cfe-commits]

mehdi_amini added a comment.

But what pipeline do we setup? For instance with ThinLTO we reduced the amount 
of passes ran during the compile phase with the expectation that more will run 
during the link, this would get fuzzy here...


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Davide Italiano via cfe-commits]

davide added a comment.

In http://reviews.llvm.org/D21006#449409, @dexonsmith wrote:

> I agree with Mehdi.  I expect `-S -flto` to give equivalent output to `-c 
> -flto`.
>
> In effect, with this change, `-flto -S` would silently ignore the `-flto` 
> flag.  That doesn't make sense to me.


I guess there's enough disagreement to hold on on this change.
My $0.02: When I ask for -S I really expect asm to be produced, but maybe it's 
just me =)
Lots of build systems have invocations of clang like:

  CFLAGS= -S
  ${CC} ${CFLAGS} blah.c -o blah.s
  sed -e ...

When CFLAGS is changed, as in:

  CFLAGS += -flto

this is generally passed globally. So, not emitting ASM in this case makes LTO 
non transparent for the build system, i.e. Makefiles need to pass `-fno-lto`, 
which might not be terrible overall, just not what I expect.


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Rafael Ávila de Espíndola via cfe-commits]

rafael added a comment.

Fair enough, let's keep it as is and try to update the build.

Cheers,
Rafael


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Rafael Espíndola via cfe-commits]

Fair enough, let's keep it as is and try to update the build.

Cheers,
Rafael
On Jun 5, 2016 9:28 PM, "Mehdi AMINI"  wrote:

> mehdi_amini added a comment.
>
> What makes me not comfortable with this change is that after that  `-c`
> would not involves codegen but `-S` would.
> Indeed I am using sometimes `-flto -S` and I expect IR, that's what is the
> most logical to me considering what -c does.
>
>
> http://reviews.llvm.org/D21006
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Duncan P. N. Exon Smith via cfe-commits]

I agree with Mehdi.  I expect `-S -flto` to give equivalent output to `-c 
-flto`.

In effect, with this change, `-flto -S` would silently ignore the `-flto` flag. 
 That doesn't make sense to me.

> On 2016-Jun-05, at 18:28, Mehdi AMINI  wrote:
> 
> mehdi_amini added a comment.
> 
> What makes me not comfortable with this change is that after that  `-c` would 
> not involves codegen but `-S` would.
> Indeed I am using sometimes `-flto -S` and I expect IR, that's what is the 
> most logical to me considering what -c does.
> 
> 
> http://reviews.llvm.org/D21006
> 
> 
> 

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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Duncan P. N. Exon Smith via cfe-commits]

dexonsmith added a subscriber: dexonsmith.
dexonsmith added a comment.

I agree with Mehdi.  I expect `-S -flto` to give equivalent output to `-c 
-flto`.

In effect, with this change, `-flto -S` would silently ignore the `-flto` flag. 
 That doesn't make sense to me.


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Mehdi AMINI via cfe-commits]

mehdi_amini added a comment.

What makes me not comfortable with this change is that after that  `-c` would 
not involves codegen but `-S` would.
Indeed I am using sometimes `-flto -S` and I expect IR, that's what is the most 
logical to me considering what -c does.


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Rafael Espíndola via cfe-commits]

OK, it prints assembly with ir in it. That doesn't apply to us since .BC is
not elf.

I guess we could just produce assembly. A .ll  cannot be used for lto.

So I guess this is OK, but please wait to see what others think.

Cheers,
Rafael
On Jun 5, 2016 7:45 PM, "Davide Italiano"  wrote:

> davide added a comment.
>
> In http://reviews.llvm.org/D21006#449380, @rafael wrote:
>
> > Can you check what GCC does?
>
>
> Sure.
>
>   $ gcc flto.c -o flto.o -flto -S && cat flto.o |head -n 15
> .file   "flto.c"
> .section.gnu.lto_.inline.513e7babbe55b1f8,"e",@progbits
> .string "x\234ca\200"
> .string "\021\006\004`d``af``b\371\240\307\300\302\300\004\026\003"
> .ascii  "\020b\001K"
> .text
> .section.gnu.lto_foo.513e7babbe55b1f8,"e",@progbits
> .string
> "x\234eP\275J\003A\020\236o\346\210Q8\365\001R\244\t!\026>C\360\021|\005A\033\341\032\373\254\\\"QA,\242\210X\004LH\341O\022[\213\004\004\021\301\"\202\235EPQ\320&\207\212\225\2363I#8\260;3\314\3673\273\036\215b\016D\276\346\2245\036\201I\017Do\201\216\bT`A\001\237\374\315\350\025[\007\362\301%!x\332F7\373\217\223\326\002im_j[{\3105\234F\277^Inl\243$L1\023\216\221\302\021\314\002'\310)\264z\365v\351\313\004H\353\316m\271\346e\353Fs\347y7\256$\234B\376\362L\007Md\r\376\320\376\331\361\023EC\247M\\5[H\350\252\320a\377\375\372\"\2319t\256\354\334W\267\n\267\236\377\277\203B\333\3104\347\273.\216C\347\356\322\360"
> .ascii
> "\223\347\241\021\rL\224\"\326$\317\bw\327\356\305,B\341P\314"
> .ascii
> "\244\321\253lN\331\263\03138\033JF\254?\235&\247\325\253~"
> .ascii
> "\341\210aJ\003\326\355\264\032[\\^\tf\027H\226\202\340\027\253"
> .ascii  "\277d'"
> .text
> .section
> .gnu.lto_.symbol_nodes.513e7babbe55b1f8,"e",@progbits
> .string "x\234ca``\020\006b\006&\206z\006\206\t\347\030@
> \324\252FE\240\b"
>
>
> http://reviews.llvm.org/D21006
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Rafael Ávila de Espíndola via cfe-commits]

rafael added a comment.

OK, it prints assembly with ir in it. That doesn't apply to us since .BC is
not elf.

I guess we could just produce assembly. A .ll  cannot be used for lto.

So I guess this is OK, but please wait to see what others think.

Cheers,
Rafael


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Davide Italiano via cfe-commits]

davide added a comment.

In http://reviews.llvm.org/D21006#449380, @rafael wrote:

> Can you check what GCC does?


Sure.

  $ gcc flto.c -o flto.o -flto -S && cat flto.o |head -n 15
.file   "flto.c"
.section.gnu.lto_.inline.513e7babbe55b1f8,"e",@progbits
.string "x\234ca\200"
.string "\021\006\004`d``af``b\371\240\307\300\302\300\004\026\003"
.ascii  "\020b\001K"
.text
.section.gnu.lto_foo.513e7babbe55b1f8,"e",@progbits
.string 
"x\234eP\275J\003A\020\236o\346\210Q8\365\001R\244\t!\026>C\360\021|\005A\033\341\032\373\254\\\"QA,\242\210X\004LH\341O\022[\213\004\004\021\301\"\202\235EPQ\320&\207\212\225\2363I#8\260;3\314\3673\273\036\215b\016D\276\346\2245\036\201I\017Do\201\216\bT`A\001\237\374\315\350\025[\007\362\301%!x\332F7\373\217\223\326\002im_j[{\3105\234F\277^Inl\243$L1\023\216\221\302\021\314\002'\310)\264z\365v\351\313\004H\353\316m\271\346e\353Fs\347y7\256$\234B\376\362L\007Md\r\376\320\376\331\361\023EC\247M\\5[H\350\252\320a\377\375\372\"\2319t\256\354\334W\267\n\267\236\377\277\203B\333\3104\347\273.\216C\347\356\322\360"
.ascii  "\223\347\241\021\rL\224\"\326$\317\bw\327\356\305,B\341P\314"
.ascii  "\244\321\253lN\331\263\03138\033JF\254?\235&\247\325\253~"
.ascii  "\341\210aJ\003\326\355\264\032[\\^\tf\027H\226\202\340\027\253"
.ascii  "\277d'"
.text
.section.gnu.lto_.symbol_nodes.513e7babbe55b1f8,"e",@progbits
.string 
"x\234ca``\020\006b\006&\206z\006\206\t\347\030@\324\252FE\240\b"


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Rafael Ávila de Espíndola via cfe-commits]

rafael added a subscriber: rafael.
rafael added a comment.

Can you check what GCC does?


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Rafael Espíndola via cfe-commits]

Can you check what GCC does?
On Jun 5, 2016 6:40 PM, "Mehdi AMINI"  wrote:

> mehdi_amini added a comment.
>
> Duncan CC for opinion.
>
>
> http://reviews.llvm.org/D21006
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Mehdi AMINI via cfe-commits]

mehdi_amini added a comment.

Duncan CC for opinion.


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Davide Italiano via cfe-commits]

davide added a comment.

In http://reviews.llvm.org/D21006#449365, @mehdi_amini wrote:

> I'm not sure it is consistent with how we handle -flto, for instance -c means 
> usually to output an object file, but adding -flto indicates to dump bitcode 
> instead.


I see two alternative approaches:

1. Force all the downstream consumers to pass -fno-lto (as I'm doing right now) 
together with -S. I don't like this option.
2. Make -flto and -S incompatible (and add a warning or an error) and have 
people that really want to emit llvm to use -emit-llvm instead?


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Mehdi AMINI via cfe-commits]

mehdi_amini added a comment.

I'm not sure it is consistent with how we handle -flto, for instance -c means 
usually to output an object file, but adding -flto indicates to dump bitcode 
instead.


http://reviews.llvm.org/D21006



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

Re: [PATCH] D21006: [Driver] Make -flto -S emit assembly

[Davide Italiano via cfe-commits]

davide updated this revision to Diff 59679.
davide added a comment.

Add a test, fix a typo.


http://reviews.llvm.org/D21006

Files:
  lib/Driver/Driver.cpp
  test/CodeGen/2009-10-20-GlobalDebug.c
  test/CodeGen/emit-asm.c
  test/CodeGenCXX/cxx-apple-kext.cpp
  test/Driver/darwin-iphone-defaults.m
  test/Driver/darwin-objc-gc.m

Index: test/Driver/darwin-objc-gc.m
===
--- test/Driver/darwin-objc-gc.m
+++ test/Driver/darwin-objc-gc.m
@@ -1,6 +1,6 @@
 // Check that we warn, but accept, -fobjc-gc for iPhone OS.
 
-// RUN: %clang -target i386-apple-darwin9 -miphoneos-version-min=3.0 -stdlib=platform -fobjc-gc -flto -S -o %t %s 2> %t.err
+// RUN: %clang -target i386-apple-darwin9 -miphoneos-version-min=3.0 -stdlib=platform -fobjc-gc -flto -S -emit-llvm -o %t %s 2> %t.err
 // RUN: FileCheck --check-prefix=IPHONE_OBJC_GC_LL %s < %t 
 // RUN: FileCheck --check-prefix=IPHONE_OBJC_GC_STDERR %s < %t.err
 
Index: test/Driver/darwin-iphone-defaults.m
===
--- test/Driver/darwin-iphone-defaults.m
+++ test/Driver/darwin-iphone-defaults.m
@@ -1,4 +1,4 @@
-// RUN: %clang -target i386-apple-darwin9 -miphoneos-version-min=3.0 -arch armv7 -stdlib=platform -flto -S -o - %s | FileCheck %s
+// RUN: %clang -target i386-apple-darwin9 -miphoneos-version-min=3.0 -arch armv7 -stdlib=platform -flto -S -emit-llvm -o - %s | FileCheck %s
 
 // CHECK: @f0() [[F0:#[0-9]+]]
 // CHECK: @__f0_block_invoke
Index: test/CodeGenCXX/cxx-apple-kext.cpp
===
--- test/CodeGenCXX/cxx-apple-kext.cpp
+++ test/CodeGenCXX/cxx-apple-kext.cpp
@@ -1,7 +1,7 @@
-// RUN: %clangxx -target x86_64-apple-darwin10 %s -flto -S -o - |\
+// RUN: %clangxx -target x86_64-apple-darwin10 %s -flto -S -emit-llvm -o - |\
 // RUN:   FileCheck --check-prefix=CHECK-NO-KEXT %s
-// RUN: %clangxx -target x86_64-apple-darwin10 %s -fapple-kext -flto -S -o - |\
-// RUN:   FileCheck --check-prefix=CHECK-KEXT %s
+// RUN: %clangxx -target x86_64-apple-darwin10 %s -fapple-kext -flto -S \
+// RUN:   -emit-llvm -o - | FileCheck --check-prefix=CHECK-KEXT %s
 
 // CHECK-NO-KEXT-NOT: _GLOBAL__D_a
 // CHECK-NO-KEXT: @is_hosted = global
Index: test/CodeGen/emit-asm.c
===
--- test/CodeGen/emit-asm.c
+++ test/CodeGen/emit-asm.c
@@ -0,0 +1,15 @@
+// Make sure ASM is emitted instead of LLVM IR.
+// RUN: %clang_cc1 %s -S -flto -o - | FileCheck %s
+
+int foo(int goo) { return goo + 42; }
+
+// CHECK:   .section  __TEXT,__text,regular,pure_instructions
+// CHECK:   .macosx_version_min 10, 11
+// CHECK:   .globl  _foo
+// CHECK:   .p2align  4, 0x90
+// CHECK: _foo:
+// CHECK:   movl  %edi, -4(%rsp)
+// CHECK:   movl  -4(%rsp), %edi
+// CHECK:   addl  $42, %edi
+// CHECK:   movl  %edi, %eax
+// CHECK:   retq
Index: test/CodeGen/2009-10-20-GlobalDebug.c
===
--- test/CodeGen/2009-10-20-GlobalDebug.c
+++ test/CodeGen/2009-10-20-GlobalDebug.c
@@ -1,16 +1,17 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang -target i386-apple-darwin10 -flto -S -g %s -o - | FileCheck %s
+// RUN: %clang -target i386-apple-darwin10 -flto -emit-llvm -S -g %s -o - |\
+// RUN:FileCheck %s
 int global;
 int main() { 
   static int localstatic;
   return 0;
 }
 
 // CHECK: !DIGlobalVariable(name: "localstatic"
 // CHECK-NOT:   linkageName:
-// CHECK-SAME:  line: 5,
+// CHECK-SAME:  line: 6,
 // CHECK-SAME:  variable: i32* @main.localstatic
 // CHECK: !DIGlobalVariable(name: "global"
 // CHECK-NOT:   linkageName:
-// CHECK-SAME:  line: 3,
+// CHECK-SAME:  line: 4,
 // CHECK-SAME:  variable: i32* @global
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1746,8 +1746,15 @@
   }
   case phases::Backend: {
 if (isUsingLTO()) {
-  types::ID Output =
-  Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC;
+  // -flto in conjunction with -S will still produce
+  // assembler, unless -emit-llvm is also specified.
+  // In that case, LLVM IR is produced instead.
+  types::ID Output;
+  if (Args.hasArg(options::OPT_S))
+Output = (Args.hasArg(options::OPT_emit_llvm)) ? types::TY_LLVM_IR :
+types::TY_PP_Asm;
+  else
+Output = types::TY_LTO_BC;
   return C.MakeAction(Input, Output);
 }
 if (Args.hasArg(options::OPT_emit_llvm)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits