[PATCH] D90048: [flang][OpenMP] Upstream lowering of OpenMP `Flush` construct

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan accepted this revision.
kiranchandramohan added inline comments.


================
Comment at: flang/lib/Lower/OpenMP.cpp:112
+              genObjectList(*ompObjectList, converter, operandRange);
+            if (auto &flushMemoryClause = std::get<
+                    std::optional<Fortran::parser::OmpFlushMemoryClause>>(
----------------
awarzynski wrote:
> SouraVX wrote:
> > kiranchandramohan wrote:
> > > Is this variable used? Would it give a warning and cause a failure in the Werror enabled bots?
> > Yes the warning comes and failure may happen. But nobody mentioned this on `FIR-dev` ?
> > But let me reconfirm? is this flag `Werror` enabled by default on all `LLVM bots` ? if so let me remove this.
> Two out three of our buildbots have `Werror` enabled for Flang: 
> * https://github.com/llvm/llvm-zorg/blob/e8ba87e92b857c14b7eb5466c4266a9e09a1f5fb/buildbot/osuosl/master/config/builders.py#L1515-L1558
> 
> As you can see one uses `clang-10` and the other `gcc-10`. Hope this helps!
Thanks @awarzynski.


================
Comment at: flang/lib/Lower/OpenMP.cpp:112
+              genObjectList(*ompObjectList, converter, operandRange);
+            if (auto &flushMemoryClause = std::get<
+                    std::optional<Fortran::parser::OmpFlushMemoryClause>>(
----------------
kiranchandramohan wrote:
> awarzynski wrote:
> > SouraVX wrote:
> > > kiranchandramohan wrote:
> > > > Is this variable used? Would it give a warning and cause a failure in the Werror enabled bots?
> > > Yes the warning comes and failure may happen. But nobody mentioned this on `FIR-dev` ?
> > > But let me reconfirm? is this flag `Werror` enabled by default on all `LLVM bots` ? if so let me remove this.
> > Two out three of our buildbots have `Werror` enabled for Flang: 
> > * https://github.com/llvm/llvm-zorg/blob/e8ba87e92b857c14b7eb5466c4266a9e09a1f5fb/buildbot/osuosl/master/config/builders.py#L1515-L1558
> > 
> > As you can see one uses `clang-10` and the other `gcc-10`. Hope this helps!
> Thanks @awarzynski.
I think we discussed the Werror setting when we were upstreaming Flang to LLVM. The LLVM default behaviour is not setting Werror. But Flang developers were of the opinion that it is a very useful flag to have to ensure that the codebase remains warning free and thus maintained at a high standard. So we decided to have a separate FLANG_WERROR flag which is by default OFF but switched ON in our buildbots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90048