Merge the middle part of the compiler #920

schweitzpgi · 2020-01-09T22:22:24Z

The Middle End

This is the set of all patches made to date to implement the middle part
of the flang compiler.  These patches contain

  - a definition of a Fortran dialect of MLIR (FIR)
  - preliminary lowering of the front-end's parse tree, expression, and
    symbol table data structures to FIR/MLIR, including the lowering of
    Fortran expressions, implicit calls, some intrinsic operations, etc.
  - translation of most of the FIR primitive operations to LLVM-IR
  - a few optimization passes
  - two test tools: bbc for testing lowering from Fortran to FIR and tco
    for testing FIR to LLVM-IR
  - a set of preliminary tests, including an expression test generator

Includes contributions from @jeanPerier, @vdonaldson, @gklimowicz and @sscalpone

This is the set of all patches made to date to implement the middle part of the flang compiler. These patches contain - a definition of a Fortran dialect of MLIR (FIR) - preliminary lowering of the front-end's parse tree, expression, and symbol table data structures to FIR/MLIR, including the lowering of Fortran expressions, implicit calls, some intrinsic operations, etc. - translation of most of the FIR primitive operations to LLVM-IR - a few optimization passes - two test tools: bbc for testing lowering from Fortran to FIR and tco for testing FIR to LLVM-IR - a set of preliminary tests, including an expression test generator

psteinfeld · 2020-01-09T22:55:40Z

Should I be able to build this? Note that I'm using Windows Subsystem for Linux, which is similar to Ubuntu. I'm using Clang 8.0.0. I get errors when I try to run CMake:

-- Performing Test CXX_SUPPORTS_FDATA_SECTIONS - Success
CMake Error at CMakeLists.txt:137 (include):
include could not find load file:

AddMLIR

-- Found PythonInterp: /usr/bin/python (found version "2.7.17")
-- Flang version: 0.1.0
-- Linking Flang driver with FIR and LLVM
-- Flang using LLVM libraries: LLVMSupport
-- Performing Test CXX_SUPPORTS_NO_DEPRECATED_COPY_FLAG
-- Performing Test CXX_SUPPORTS_NO_DEPRECATED_COPY_FLAG - Failed
-- Performing Test CXX_SUPPORTS_NO_STRING_CONVERSION_FLAG
-- Performing Test CXX_SUPPORTS_NO_STRING_CONVERSION_FLAG - Success
-- Performing Test CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG
-- Performing Test CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG - Success
CMake Error at include/optimizer/CMakeLists.txt:2 (include):
include could not find load file:

AddMLIR

CMake Error at include/optimizer/CMakeLists.txt:6 (mlir_tablegen):
Unknown CMake command "mlir_tablegen".

-- Configuring incomplete, errors occurred!

kiranchandramohan · 2020-01-09T22:57:01Z

Same errors are there in the CI as well. Is there a change in the build instructions?
https://cloud.drone.io/flang-compiler/f18/549/1/2

klausler · 2020-01-09T23:02:39Z

lib/decimal/binary-to-decimal.cc

@@ -312,6 +312,15 @@ void BigRadixFloatingPointNumber<PREC,
  }
 }

+#define INSTANTIATE_WALDO(X) \


What's this, here and in decimal-to-binary.cc?

These are hacks to get these files to build correctly on my Mac.

What was broken? Were you getting unresolved external names?

It was a long time ago. But as far as I remember, yes, the builds started failing after this code was introduced with link-time failures.

FWIW, I keep my Mac updated to the "latest and greatest" Apple tools, so there have been occasions where I stumble into issues.

If we need this to avoid errors could we name it something more descriptive and add a comment saying why it is here? I don't understand how the name INSTANTIATE_WALDO relates to what this macro does.

This might be a work-around to a compiler bug with template instantiation in a specific environment. Please confirm that the bug persists; if it does, other work-arounds can be tried.

It appears to have been a transitory issue. This code will be removed.

vjayathirtha-nv · 2020-01-09T23:18:48Z

I was able to build using these instructions.
https://github.com/schweitzpgi/f18/tree/mono#monorepo-now-contains-mlir

schweitzpgi · 2020-01-09T23:24:54Z

Same errors are there in the CI as well. Is there a change in the build instructions?
https://cloud.drone.io/flang-compiler/f18/549/1/2

This merge introduces true dependencies between the F18 project and MLIR, LLVM. In-tree and out-of-tree builds were tested.

Varun posted a link to the directions. https://github.com/schweitzpgi/f18/tree/mono#monorepo-now-contains-mlir

sscalpone · 2020-01-10T02:14:19Z

@schweitzpgi Does this PR need flang-compiler/f18-llvm-project or does it compile against the master branch of llvm/llvm-project?

schweitzpgi · 2020-01-10T15:41:26Z

@schweitzpgi Does this PR need flang-compiler/f18-llvm-project or does it compile against the master branch of llvm/llvm-project?

It needs the patches found in flang-compiler/f18-llvm-project on the mono branch.

RichBarton-Arm · 2020-01-10T16:54:22Z

I'm very excited that Flang may soon generate some code, but I am strongly against this being pushed as-is to F18.

This pull request contains one patch comprising 23000 lines of code. This patch is far too big. Contributing FIR in this way will hide all the reasoning behind the changes and makes review incredibly difficult. It will also make bisection impossible for this large part of the compiler.

I also question why had Peter create a script to maintain the git history of F18 when it goes into the LLVM monorepo only to have this patch at the top of said history with 23000 LOC in it?

If we accept this patch into Flang before the patches in flang-compiler/f18-llvm-project are accepted into the monorepo, then llvm-project/flang will not build with the other components of llvm even following Flang's own out-of-tree build instructions. Given the (very reasonable) scrutiny F18 is under in the LLVM community right now for not looking and smelling like an LLVM project this is going to really hurt our credibility.

I am also unsure of how complete the implementation is and of how well tested it is. The commit message describes the tests as 'preliminary' so how well can we expect this code to work? There are 26 tests in this patch, which to me indicates it is not well covered by testing.

I think that this PR needs to be broken down into a logical sequence of patches that builds up the FIR capability from scratch. This way, we can review this important code in digestable chunks and ensure that we are testing well the functionality as we build it up. Arm are very happy to conduct these reviews.

DavidTruby · 2020-01-10T14:37:56Z

CMakeLists.txt

@@ -8,95 +8,384 @@



Would it be possible to get these CMake changes as a separate PR? They seem largely orthogonal to merging the middle-end and would be easier to review separately

DavidTruby · 2020-01-10T14:52:52Z

include/optimizer/.clang-format

@@ -0,0 +1,2 @@
+BasedOnStyle: LLVM
+AlwaysBreakTemplateDeclarations: Yes


This file is missing a newline at the end

DavidTruby · 2020-01-10T14:57:20Z

include/optimizer/Analysis/IteratedDominanceFrontier.h

+#include "mlir/Support/LLVM.h"
+
+namespace mlir {
+class Block;


This class is already declared in Block.h

DavidTruby · 2020-01-10T14:57:57Z

include/optimizer/Analysis/IteratedDominanceFrontier.h

+
+namespace mlir {
+class Block;
+class DominanceInfo;


This class is already declared in Dominance.h

DavidTruby · 2020-01-10T15:00:06Z

include/optimizer/Analysis/IteratedDominanceFrontier.h

+  /// not include blocks where any phi insertion would be dead.
+  ///
+  /// Note: This set *must* live for the entire lifetime of the IDF calculator.
+


Nit: stray newline

DavidTruby · 2020-01-10T17:48:56Z

lib/optimizer/Transforms/CSE.cpp

+    unsigned hashOps;
+    if (op->isCommutative()) {
+      std::vector<void *> vec;
+      for (auto i = op->operand_begin(), e = op->operand_end(); i != e; ++i)


Suggested change

for (auto i = op->operand_begin(), e = op->operand_end(); i != e; ++i)

for (auto i : op->getOperands())

DavidTruby · 2020-01-10T17:52:24Z

tools/bbc/.clang-format

@@ -0,0 +1,2 @@
+BasedOnStyle: LLVM
+AlwaysBreakTemplateDeclarations: Yes


Nit: missing new line at end of file

DavidTruby · 2020-01-10T17:52:57Z

tools/tco/.clang-format

@@ -0,0 +1,2 @@
+BasedOnStyle: LLVM
+AlwaysBreakTemplateDeclarations: Yes


Nit: missing new line at end of file

DavidTruby · 2020-01-10T17:53:34Z

tools/tco/tco.cpp

+  llvm::InitLLVM y(argc, argv);
+  (void)y;


Since we're using C++17 we can just mark this variable ``[[maybe_unused]]` for clarity

DavidTruby · 2020-01-10T17:54:03Z

tools/bbc/bbc.cpp

+  InitLLVM y(argc, argv);
+  (void)y;


Since we're using C++17 we can just mark this variable [[maybe_unused]] for clarity

DavidTruby · 2020-01-10T18:14:22Z

Just to briefly add to what @RichBarton-Arm said above, I have done my best to review this but I don't feel that I have been close to as thorough as such a significant part of the codebase deserves. It would be much easier to review to a good standard if this were split into multiple PRs.

- add newlines to .clang-format - remove old school (void) casts and replace with new school [[maybe_unused]] - remove unneeded headers - mlir-tablegen now supports #ifndef - remove a newline - remove a few lines - make code more succinct

kiranchandramohan

Thanks for making these changes available. I have a few comments.
Are there tests for lowering to LLVM-IR dialect?
The LLVM-IR in the first comment/description of this PR refers to the dialect right?

As others have said it would be easier to have a look if changes are organized as a sequence of commits.

kiranchandramohan · 2020-01-10T10:52:32Z

tools/tco/tco.cpp

+
+  if (!owningRef) {
+    errs() << "Error can't load file " << ClInput << '\n';
+    return 2;


Can these return error values be values from an enum or preprocessor vals?
I see that similar values are used in the mlir toy examples.

This is a rough draft of a test tool. It needs more work to make it useful for lit, etc.

kiranchandramohan · 2020-01-10T11:57:39Z

lib/optimizer/.clang-format

@@ -0,0 +1,2 @@
+BasedOnStyle: LLVM
+AlwaysBreakTemplateDeclarations: Yes


kiranchandramohan · 2020-01-10T18:23:59Z

include/optimizer/FIROps.h

+  using Op::Op;
+  using Op::print;
+
+  static llvm::StringRef getOperationName() { return "fir.global"; }


Already declared using llvm::StringRef above. Also in several places below for both StringRef and ArrayRef.

kiranchandramohan · 2020-01-10T18:52:52Z

lib/lower/convert-expr.cc

+namespace Br = Fortran::lower;
+namespace Co = Fortran::common;
+namespace Ev = Fortran::evaluate;
+namespace L = llvm;
+namespace M = mlir;
+namespace Pa = Fortran::parser;
+namespace Se = Fortran::semantics;


Can these namespace abbreviations be in a single file and included wherever necessary?

kiranchandramohan · 2020-01-10T18:55:56Z

lib/lower/convert-expr.cc

+#define TODO()                                                                 \
+  assert(false);                                                               \
+  return {}


There seems to be three TODOs with slightly different meanings. Can a prefix or suffix be added to signify the difference?

The TODO macros are ephemeral. I'm not sure there is great value in standardizing them.

kiranchandramohan · 2020-01-10T22:21:58Z

lib/optimizer/CodeGen.cpp

+    auto a = operands[0];
+    auto loc = boxaddr.getLoc();
+    auto ty = convertType(boxaddr.getType());
+    if (auto argty = boxaddr.val()->getType().dyn_cast<BoxType>()) {


kiranchandramohan · 2020-01-10T22:29:41Z

lib/optimizer/CodeGen.cpp

+                  L::ArrayRef<OperandTy> destOperands,
+                  M::ConversionPatternRewriter &rewriter) const override {
+    selectMatchAndRewrite<fir::SelectOp>(lowering, op, operands, destinations,
+                                         destOpera


FIXME or TODO?
Could you add some details?

We (obviously) haven't spent enough time in documenting our interfaces in the project. It is on the list of things to be done.

kiranchandramohan · 2020-01-10T22:34:01Z

lib/optimizer/FIROps.cpp

+
+// LoopOp
+
+void LoopOp::build(mlir::Builder *builder, mlir::OperationState &result,


mlir:: -> M:: for consistency.

kiranchandramohan · 2020-01-10T22:47:14Z

lib/optimizer/FIRType.cpp

+           ty.dyn_cast<ReferenceType>() || ty.dyn_cast<TypeDescType>());
+}
+
+bool verifySameLists(L::ArrayRef<RecordType::TypePair> a1,


Wouldn't the == on ArrayRef work?
https://github.com/llvm/llvm-project/blob/6c2151bf4c829958891e65a4cc396daa6d308eb0/llvm/include/llvm/ADT/ArrayRef.h#L533

Yes, that's fine for how this test is written. We may need a more sophisticated algorithm here. It's not entirely clear at the moment.

kiranchandramohan · 2020-01-10T23:01:07Z

lib/optimizer/StdConverter.cpp

+                  L::ArrayRef<M::Block *> destinations,
+                  L::ArrayRef<OperandTy> destOperands,
+                  M::ConversionPatternRewriter &rewriter) const override {
+    auto selecttype = M::cast<SelectTypeOp>(op);


schweitzpgi · 2020-01-11T00:04:29Z

Thanks for making these changes available. I have a few comments.

Thanks for taking a look at the project, Kiran.

Are there tests for lowering to LLVM-IR dialect?

There are some. However, the project is very much in active development and still being written.

The LLVM-IR in the first comment/description of this PR refers to the dialect right?

Yes. FIR is lowered to the LLVM-IR dialect of MLIR.

And, not exactly. The meaning of the statement was that (most) patches have already been upstreamed to the MLIR project so that the LLVM-IR dialect will generate a file that contains LLVM IR. (I'm working on the last one now.)

sscalpone · 2020-01-11T00:27:17Z

@schweitzpgi Does this PR need flang-compiler/f18-llvm-project or does it compile against the master branch of llvm/llvm-project?

It needs the patches found in flang-compiler/f18-llvm-project on the mono branch.

I was able to build this PR out-of-tree with a fresh llvm from flang-compiler/f18-llvm-project. I was not able to build llvm with shared libraries owing to compilation problems when building mlir. I also disabled ZLIB when building llvm because I am unable to share the resulting llvm install with zlib between redhat and ubuntu systems.

sscalpone · 2020-01-11T00:31:41Z

As others have said it would be easier to have a look if changes are organized as a sequence of commits.

The commits are available in https://github.com/schweitzpgi/f18

By the slice or by the pie, it's all the same pizza.

sscalpone · 2020-01-11T00:41:12Z

If we accept this patch into Flang before the patches in flang-compiler/f18-llvm-project are accepted into the monorepo, then llvm-project/flang will not build with the other components of llvm even following Flang's own out-of-tree build instructions. Given the (very reasonable) scrutiny F18 is under in the LLVM community right now for not looking and smelling like an LLVM project this is going to really hurt our credibility.

+1 this patch should not be merged until it builds against the monorepo.

psteinfeld · 2020-01-12T17:05:00Z

I was able to build using these instructions.
https://github.com/schweitzpgi/f18/tree/mono#monorepo-now-contains-mlir

Thanks, Varun.

I tried following those instructions. I'm a newbie to all of this, and there were things that puzzled or blocked me. The title says "Monorepo now contains MLIR", but I don't know what that means or why it's important.

Also, there are two options, an "in-tree" build, and an "out-of-tree" build. I'm not sure what the differences are or why I should choose one over the other.

I guessed that I wanted an "in-tree" build. But when I tried both of the "git clone" commands, I got messages saying "Permission denied".

CarolineConcatto · 2020-01-12T18:31:14Z

include/flang/Version.h

+  /// Retrieves the repository revision number (or identifier) from which
+  /// LLVM was built.
+  ///
+  /// If Clang and LLVM are in the same repository, this returns the same


Is it Clang or Flang. Why would we need Clang here?

CarolineConcatto · 2020-01-12T18:32:31Z

include/flang/Version.h

+  std::string getLLVMRevision();
+
+  /// Retrieves the full repository version that is an amalgamation of
+  /// the information in getClangRepositoryPath() and getClangRevision().


I guess the comments where I see Clang should be Flang now. Correct?

CarolineConcatto · 2020-01-12T18:34:24Z

include/flang/Version.inc.in

@@ -0,0 +1,5 @@
+#define FLANG_VERSION @CLANG_VERSION@


Why Flang is the same as Clang?

CarolineConcatto · 2020-01-12T22:33:16Z

lib/lower/cfg-builder.h

+      if (e.subs) {
+        return false;
+      }
+      switch (e.cfg) {


This does not need a default case?

Not when all the values of the enum are explicitly covered by the cases.

CarolineConcatto · 2020-01-12T22:35:21Z

lib/lower/cfg-builder.h

+      (void)rc; // for release build
+      return;
+    }
+    for (auto *s : *iter->second)


Should *iter be tested for null first?

It would be incorrect to add a nullptr to the map.

CarolineConcatto · 2020-01-12T23:22:46Z

lib/lower/convert-type.cc

+}
+
+#undef TODO
+#define TODO()                                                                 \


is there a reason for the back-slash ?

It's a C preprocessor line continuation. TODO() expands to the following two lines.

CarolineConcatto · 2020-01-12T23:31:28Z

tools/tco/tco.cpp

+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;


I've notice that the first files you use llvm::, and by the end namespace started to be used.
Is there a reason for that? If not, why not use using namespace llvm for all?

CarolineConcatto · 2020-01-12T23:34:28Z

tools/tco/CMakeLists.txt

@@ -0,0 +1,43 @@
+#===-- tools/tco/CMakeLists.txt --------------------------------------------===#


Is there a need for header here? I may be wrong, but I believe CMakeLists.txt does not need the header.

Appears the header rewrites got overly exuberant.

CarolineConcatto · 2020-01-12T23:38:31Z

tools/tco/tco.cpp

+}
+
+int main(int argc, char **argv) {
+  [[maybe_unused]] llvm::InitLLVM y(argc, argv);


Do we need that when we have namespace llvm?

CarolineConcatto · 2020-01-12T23:45:45Z

test/lower/expr-test-generator.cc

+
+std::optional<Type> Operation::GetResultTypeForArguments(
+    Type lhs, Type rhs) const {
+  // See Fortran 2018 table 10.2 and section 10.1.9.3


I believe in the future would be nice to write the reason and reference the standard for more info.
Instead of only adding the reference to the standard

kiranchandramohan

I was able to build (with 4 compiler warnings) this PR using the build instructions provided in the page mentioned by @vjayathirtha-nv. If we are to push this patch before the llvm submission then the Readme file should be updated with the new build instructions.
Was not able to find a way to run the existing tests or the new tests that come as part of the PR. ninja test, ninja check, ninja check-all did not work. There is no test directory hence the old way to run tests ($HOME/cmake-3.11.0-rc4/build/bin/ctest --force-new-ctest-process) also did not work. Is this expected only to work after the lit changes?
I guess some of the changes for FIR will actually be moving to the mlir directories. What is the plan for this? Is this move planned only after the project is mature?
A few more comments inline.

kiranchandramohan · 2020-01-11T16:42:37Z

include/optimizer/Attribute.h

@@ -0,0 +1,136 @@
+//===-- include/fir/Attribute.h ---------------------------------*- C++ -*-===//


Mismatch with real file path. I guess this comment applies to all headers in this directory. Probably a renaming happened sometime.

kiranchandramohan · 2020-01-11T16:45:24Z

include/optimizer/Attribute.h

+//
+//===----------------------------------------------------------------------===//
+
+#ifndef OPTIMIZER_FIRATTRIBUTE_H


Should the filename be FirAttribute.h or should this define be ATTRIBUTE_H?

kiranchandramohan · 2020-01-11T17:19:11Z

include/optimizer/FIROpsSupport.h

+  // Should we also auto-detect that the called function is pure if its
+  // arguments are not references?  For now, rely on a "pure" attribute.
+  if (auto call = dyn_cast<fir::CallOp>(op))
+    return bool(call.getAttr("pure"));


Is the creation of a bool necessary here? (Note: it is not created in the function above)

kiranchandramohan · 2020-01-12T00:08:33Z

include/optimizer/FIRType.h

+  FIR_TYPEDESC,
+};
+
+bool isa_fir_type(mlir::Type);


Name for the parameter for consistency with the ones below?

kiranchandramohan · 2020-01-13T04:35:22Z

lib/optimizer/InternalNames.cpp

+  }
+  std::int64_t result;
+  uniq.substr(init, i).getAsInteger(10, result);
+  return result;


compilation warning here for result maybe-uninitialized.

kiranchandramohan · 2020-01-13T04:36:40Z

lib/optimizer/KindMapping.cpp

+    KindTy kind;
+    if (parseCode(code, srcPtr) || parseInt(kind, srcPtr))
+      return badMapString(srcPtr);
+    if (code == 'a' || code == 'i' || code == 'l') {


compilation warning here for code maybe-uninitialized.

kiranchandramohan · 2020-01-13T04:37:52Z

lib/optimizer/KindMapping.cpp

+      LLVMTypeID id;
+      if (parseColon(srcPtr) || parseTypeID(id, srcPtr))
+        return badMapString(srcPtr);
+      floatMap[code][kind] = id;


compilation warning here for id maybe-uninitialized.

kiranchandramohan · 2020-01-13T04:43:57Z

lib/optimizer/CodeGen.cpp

+};
+
+/// return true if all `Value`s in `operands` are `ConstantOp`s
+bool allConstants(OperandTy operands) {


compilation warning here for unused function. (I guess it is not used in Release builds)

Answer some comments from flang-compiler#920: - Use std::unique_ptr for IntrinsicLibrary::Implementation instead of a simple "owning pointer" comment. - Remove two "#if 0" dead code pieces

kiranchandramohan · 2020-01-13T14:02:30Z

As others have said it would be easier to have a look if changes are organized as a sequence of commits.

The commits are available in https://github.com/schweitzpgi/f18

By the slice or by the pie, it's all the same pizza.

:)

As I understand at lof of the changes in this PR was already reviewed in two separate PRs (links given below). If the new changes were separate commits then we only need to look at the new changes. Also, this new PR which is a single commit gives the impression that this a single piece of unreviewed code to a person who has not been following the f18 project.
#720
#696
PS: I understand that this might be complicated by the fast-moving MLIR project.

psteinfeld · 2020-01-13T19:19:35Z

I was able to build using these instructions.
https://github.com/schweitzpgi/f18/tree/mono#monorepo-now-contains-mlir

Thanks, Varun.

I tried following those instructions. I'm a newbie to all of this, and there were things that puzzled or blocked me. The title says "Monorepo now contains MLIR", but I don't know what that means or why it's important.

Also, there are two options, an "in-tree" build, and an "out-of-tree" build. I'm not sure what the differences are or why I should choose one over the other.

I guessed that I wanted an "in-tree" build. But when I tried both of the "git clone" commands, I got messages saying "Permission denied".
I'm currently building, and there are no errors so far. My initial problem was that the "git clone" commands. I modified them to use the URLs supplied by GitHub as follows:
git clone https://github.com/schweitzpgi/f18.git
git clone https://github.com/flang-compiler/f18-llvm-project.git

psteinfeld · 2020-01-13T20:56:02Z

I was able to build using these instructions.
https://github.com/schweitzpgi/f18/tree/mono#monorepo-now-contains-mlir

Thanks, Varun.
I tried following those instructions. I'm a newbie to all of this, and there were things that puzzled or blocked me. The title says "Monorepo now contains MLIR", but I don't know what that means or why it's important.
Also, there are two options, an "in-tree" build, and an "out-of-tree" build. I'm not sure what the differences are or why I should choose one over the other.
I guessed that I wanted an "in-tree" build. But when I tried both of the "git clone" commands, I got messages saying "Permission denied".
I'm currently building, and there are no errors so far. My initial problem was that the "git clone" commands. I modified them to use the URLs supplied by GitHub as follows:
git clone https://github.com/schweitzpgi/f18.git
git clone https://github.com/flang-compiler/f18-llvm-project.git

Here's some performance information about my build. Note that I'm using a Windows laptop with 6 cores and 32 GB of RAM. I'm running Windows 10 using Windows Subsystem for Linux with Ubuntu installed. I used version 8.3 of the GNU compiler. I was also hard-wired into NVIDIA's network at the office. It took me between 9 and 10 minutes to clone the LLVM repository. The actual build (using Ninja) took 57 minutes during which time all of the processors on my machine were showing close to full utilization. Also, a rebuild after changing one file takes me 2.5 minutes. For my old build of just f18, this took about 1 minute.

…Perier/f18 into middle-end

Conflicts: include/flang/CMakeLists.txt tools/tco/tco.cpp

schweitzpgi · 2020-01-15T19:34:42Z

This merge will not be taking place at this time.

schweitzpgi requested review from sscalpone, gklimowicz, ichoyjx, jeanPerier, tskeith, vjayathirtha-nv, klausler, vdonaldson and psteinfeld January 9, 2020 22:22

klausler reviewed Jan 9, 2020

View reviewed changes

DavidTruby suggested changes Jan 10, 2020

View reviewed changes

respond to review comments:

d5a4982

- add newlines to .clang-format - remove old school (void) casts and replace with new school [[maybe_unused]] - remove unneeded headers - mlir-tablegen now supports #ifndef - remove a newline - remove a few lines - make code more succinct

kiranchandramohan reviewed Jan 10, 2020

View reviewed changes

CarolineConcatto reviewed Jan 12, 2020

View reviewed changes

kiranchandramohan reviewed Jan 13, 2020

View reviewed changes

Use std::unique_ptr for IntrinsicLibrary implementation

6effbc2

Answer some comments from flang-compiler#920: - Use std::unique_ptr for IntrinsicLibrary::Implementation instead of a simple "owning pointer" comment. - Remove two "#if 0" dead code pieces

jeanPerier mentioned this pull request Jan 13, 2020

Use std::unique_ptr for IntrinsicLibrary implementation schweitzpgi/obsolete-f18#35

Merged

schweitzpgi added 3 commits January 13, 2020 13:12

review comments

ec85e0c

Merge branch 'jpr-pimp-intrinsic-lowering' of https://github.com/jean…

79fffc5

…Perier/f18 into middle-end

review comments from Caroline

a8a4cf3

Conflicts: include/flang/CMakeLists.txt tools/tco/tco.cpp

schweitzpgi closed this Jan 15, 2020

schweitzpgi deleted the middle-end branch January 15, 2020 19:34

		@@ -0,0 +1,2 @@
		BasedOnStyle: LLVM
		AlwaysBreakTemplateDeclarations: Yes

	for (auto i = op->operand_begin(), e = op->operand_end(); i != e; ++i)
	for (auto i : op->getOperands())


		// LoopOp

		void LoopOp::build(mlir::Builder *builder, mlir::OperationState &result,

		@@ -0,0 +1,43 @@
		#===-- tools/tco/CMakeLists.txt --------------------------------------------===#

		@@ -0,0 +1,136 @@
		//===-- include/fir/Attribute.h ---------------------------------- C++ --===//

Merge the middle part of the compiler #920

Merge the middle part of the compiler #920

Conversation

schweitzpgi commented Jan 9, 2020

psteinfeld commented Jan 9, 2020

kiranchandramohan commented Jan 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schweitzpgi Jan 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjayathirtha-nv commented Jan 9, 2020 • edited Loading

schweitzpgi commented Jan 9, 2020

sscalpone commented Jan 10, 2020

schweitzpgi commented Jan 10, 2020

RichBarton-Arm commented Jan 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidTruby commented Jan 10, 2020

kiranchandramohan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schweitzpgi commented Jan 11, 2020

sscalpone commented Jan 11, 2020

sscalpone commented Jan 11, 2020

sscalpone commented Jan 11, 2020

psteinfeld commented Jan 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiranchandramohan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiranchandramohan commented Jan 13, 2020 • edited Loading

psteinfeld commented Jan 13, 2020

psteinfeld commented Jan 13, 2020

schweitzpgi commented Jan 15, 2020

schweitzpgi Jan 9, 2020 •

edited

Loading

vjayathirtha-nv commented Jan 9, 2020 •

edited

Loading

kiranchandramohan left a comment •

edited

Loading

kiranchandramohan commented Jan 13, 2020 •

edited

Loading