[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

Thanks for this patch. This is one big progress to use `flang-new` for real workloads. Mostly looks good to me. I have several minor comments. If supporing `-Ofast` requires more changes, I prefer to push forward with the current patch.

summary: fronted -> frontend

Confirmed with the following case:

  program m
    integer :: x = 1, y = 2, z = 0
    call add(x, y, z)
    print *, z
  contains
    subroutine add(a, b, c)
      integer :: a, b, c
      integer :: i
      do i = 1, 10
        c = c + a + b
      enddo
    end
  end

  $ flang-new -fc1 -emit-llvm test.f90 -O3
  $ cat test.ll 
  ; ModuleID = 'FIRModule'
  source_filename = "FIRModule"
  target triple = "aarch64-unknown-linux-gnu"
  
  @_QFEz = internal unnamed_addr global i32 0
  @_QQcl.2E2F746573742E66393000 = linkonce constant [11 x i8] c"./test.f90\00"
  
  define void @_QQmain() local_unnamed_addr !dbg !3 {
    %1 = load i32, ptr @_QFEz, align 4, !dbg !7
    %2 = add i32 %1, 30, !dbg !7
    store i32 %2, ptr @_QFEz, align 4, !dbg !7
    %3 = tail call ptr @_FortranAioBeginExternalListOutput(i32 -1, ptr nonnull @_QQcl.2E2F746573742E66393000, i32 5), !dbg !12
    %4 = tail call i1 @_FortranAioOutputInteger32(ptr %3, i32 %2), !dbg !13
    %5 = tail call i32 @_FortranAioEndIoStatement(ptr %3), !dbg !12
    ret void, !dbg !14
  }
  ; Function Attrs: argmemonly nofree norecurse nosync nounwind
  define void @_QFPadd(ptr nocapture readonly %0, ptr nocapture readonly %1, ptr nocapture %2) local_unnamed_addr #0 !dbg !9 {
     ...



================
Comment at: clang/include/clang/Driver/Options.td:732
+def O_flag : Flag<["-"], "O">, Flags<[CC1Option,FC1Option]>, Alias<O>, AliasArgs<["1"]>;
 def Ofast : Joined<["-"], "Ofast">, Group<O_Group>, Flags<[CC1Option]>;
 def P : Flag<["-"], "P">, Flags<[CC1Option,FlangOption,FC1Option]>, Group<Preprocessor_Group>,
----------------
Will enabling Ofast require more changes in the flang frontend? If yes, it is OK to support it in another patch later.


================
Comment at: flang/include/flang/Frontend/CodeGenOptions.def:30
+
+VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
+
----------------
I saw `clang/include/clang/Basic/CodeGenOptions.def` has the following:
```
VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is specified.
```

Do `-Os` and `-Oz` need extra processing in flang drivers? If yes, it is OK to support it in another patch later.


================
Comment at: flang/lib/Frontend/CodeGenOptions.cpp:8
+//===----------------------------------------------------------------------===//
+
+#include "flang/Frontend/CodeGenOptions.h"
----------------
Miss the following?
```
//
// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
//
//===----------------------------------------------------------------------===//
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128043