[clang] -foptimization-record-file= should imply -fsave-optimization-record

[Quolyk]

Summary

This is my first attempt to contribute to llvm. I'm trying to implement this https://bugs.llvm.org/show_bug.cgi?id=33670.

[Quolyk]

Added 1 test, I guess it's sufficient

[malaperle]

Hi! You put "[clangd]" in the title, but I don't believe it's related to clangd but rather just "clang"?

[Quolyk]

In D39834#930165, @malaperle wrote:

Hi! You put "[clangd]" in the title, but I don't believe it's related to clangd but rather just "clang"?

I thought patch to clang option is supposed to be marked as "clangd".

[JDevlieghere]

I think you can achieve the same result with less code by checking for the flag's presence higher up, where currently OPT_fsave_optimization_record is handled (Clang.cpp:4329). Something like:

if (Args.hasFlag(options::OPT_fsave_optimization_record,
                 options::OPT_fno_save_optimization_record, false) || 
    Args.hasFlag(options::OPT_foptimization_record_file_EQ, 
                 options::OPT_fno_save_optimization_record, false)) {

The test looks good to me.

[JDevlieghere]

This also ensures that if fno_save_optimization_record is specified, you don't overwrite it by setting the file. This is definitely something you'd want to add to your test case.

[JDevlieghere]

Thanks Dmitry, this LGTM!

PS: Let me know if you don't have commit access and want me to commit it for you.

[Quolyk]

In D39834#959500, @JDevlieghere wrote:

Thanks Dmitry, this LGTM!

PS: Let me know if you don't have commit access and want me to commit it for you.

I don't have commit access, please commit. Thanks for code review.

lib/Driver/ToolChains/Clang.cpp

if (A) {
           F += "-";
           F += JA.getOffloadingArch();
         }
       }
 
       llvm::sys::path::replace_extension(F, "opt.yaml");
       CmdArgs.push_back(Args.MakeArgString(F));
     }
+  } else {
+    if (const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ)) {
+      CmdArgs.push_back("-opt-record-file");
+      CmdArgs.push_back(A->getValue());
+    }
   }
 
   bool RewriteImports = Args.hasFlag(options::OPT_frewrite_imports,
                                      options::OPT_fno_rewrite_imports, false);
   if (RewriteImports)
     CmdArgs.push_back("-frewrite-imports");
 
   // Enable rewrite includes if the user's asked for it or if we're generating

test/Driver/opt-record.c

 // RUN: %clang -### -S -o FOO -fsave-optimization-record %s 2>&1 | FileCheck %s
 // RUN: %clang -### -c -o FOO -fsave-optimization-record %s 2>&1 | FileCheck %s
 // RUN: %clang -### -c -o FOO.o -fsave-optimization-record %s 2>&1 | FileCheck %s
 // RUN: %clang -### -save-temps -S -o FOO -fsave-optimization-record %s 2>&1 | FileCheck %s
 // RUN: %clang -### -save-temps -c -o FOO.o -fsave-optimization-record %s 2>&1 | FileCheck %s
 // RUN: %clang -### -c -fsave-optimization-record %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O
 // RUN: %clang -### -save-temps -c -fsave-optimization-record %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O
 // RUN: %clang -### -fsave-optimization-record %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O
 // RUN: %clang -### -S -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -fsave-optimization-record -x cuda -nocudainc -nocudalib %s 2>&1 | FileCheck %s -check-prefix=CHECK-NO-O -check-prefix=CHECK-CUDA-DEV
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
+// RUN: %clang -### -S -o FOO -foptimization-record-file=BAR.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ
 
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
 // CHECK-NO-O: "-cc1"
 // CHECK-NO-O-DAG: "-opt-record-file" "opt-record.opt.yaml"
 // CHECK-CUDA-DEV-DAG: "-opt-record-file" "opt-record-cuda-{{nvptx64|nvptx}}-nvidia-cuda-sm_20.opt.yaml"
 
 // CHECK-EQ: "-cc1"
 // CHECK-EQ: "-opt-record-file" "BAR.txt"