[Driver] Make -flto -S emit assembly

[davide]

Summary

Currently clang emits LLVM IR if -flto -S is specified.
This confuses build systems, which generally want assembly to emitted if -S is specified (as it happens with non-LTO).
I propose to change the driver so that -flto -S emits assembly. If somebody wants to override the behavior can specify -S -flto -emit-llvm

[davide]

Add a test, fix a typo.

[mehdi_amini]

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.

[davide]

In 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?

[mehdi_amini]

Duncan CC for opinion.

[• rafael]

Can you check what GCC does?

[davide]

In 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?&OX\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"

[• rafael]

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

[mehdi_amini]

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.

[dexonsmith]

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.

[• rafael]

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

Cheers,
Rafael

[davide]

In 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.

[mehdi_amini]

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...

lib/Driver/Driver.cpp

     if (Args.hasArg(options::OPT_module_file_info))
       return C.MakeAction<CompileJobAction>(Input, types::TY_ModuleFile);
     if (Args.hasArg(options::OPT_verify_pch))
       return C.MakeAction<VerifyPCHJobAction>(Input, types::TY_Nothing);
     return C.MakeAction<CompileJobAction>(Input, types::TY_LLVM_BC);
   }
   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<BackendJobAction>(Input, Output);
     }
     if (Args.hasArg(options::OPT_emit_llvm)) {
       types::ID Output =
           Args.hasArg(options::OPT_S) ? types::TY_LLVM_IR : types::TY_LLVM_BC;
       return C.MakeAction<BackendJobAction>(Input, Output);
     }
     return C.MakeAction<BackendJobAction>(Input, types::TY_PP_Asm);

test/CodeGen/2009-10-20-GlobalDebug.c

 // 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

test/CodeGen/emit-asm.c

+// 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

test/CodeGenCXX/cxx-apple-kext.cpp

-// 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
 // CHECK-NO-KEXT: @_ZTI3foo = {{.*}} @_ZTVN10__cxxabiv117
 // CHECK-NO-KEXT: call i32 @__cxa_atexit({{.*}} @_ZN3fooD1Ev
 // CHECK-NO-KEXT: declare i32 @__cxa_atexit
 
 // CHECK-KEXT: @_ZTV3foo = 

test/Driver/darwin-iphone-defaults.m

-// 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
 // CHECK: void @f1
 // CHECK-NOT: msgSend_fixup_alloc
 // CHECK: OBJC_SELECTOR_REFERENCES
 
 int f0() {

test/Driver/darwin-objc-gc.m

 // 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
 
 // IPHONE_OBJC_GC_LL: define void @f0
 // IPHONE_OBJC_GC_LL-NOT: objc_assign_ivar
 // IPHONE_OBJC_GC_LL: }
 
 // IPHONE_OBJC_GC_STDERR: warning: Objective-C garbage collection is not supported on this platform, ignoring '-fobjc-gc'
 
 @interface A {
 @public
  id x;
 }
 @end
 
 void f0(A *a, id x) { a->x = x; }