[flang-commits] [PATCH] D133561: [flang] Add Sparc support to Optimizer/CodeGen/Target.cpp

[Diana Picus via Phabricator via flang-commits]

rovka accepted this revision.
rovka added a comment.
This revision is now accepted and ready to land.

In D133561#3808337 <https://reviews.llvm.org/D133561#3808337>, @ro wrote:

> In D133561#3792286 <https://reviews.llvm.org/D133561#3792286>, @rovka wrote:
>
>> This doesn't have any tests, could you please add some? If memory holds, you should be able to add some RUN/CHECK lines here: https://github.com/llvm/llvm-project/blob/main/flang/test/Fir/target-rewrite-complex.fir
>
> The new testcase `PASS`es on `sparcv9-sun-solaris2.11` and `sparc64-unknown-linux-gnu`.  However, for 32-bit sparc it runs into an assertion failure
>
>   Assertion failed: llvm::all_of(types, [](Type t) { return t; }) && "attempting to construct a TypeRange with null types", file /vol/llvm/src/llvm-project/local/mlir/lib/IR/TypeRange.cpp, line 20
>
> and I don't yet see what's wrong.  Any suggestions or do I need to run a full Debug build to investigate?

Not sure what the issue is there. I'm actually a bit surprised that it works for i386. When I tried to enable flang for armv7, MLIR didn't have very good support for 32-bit platforms, so I left it at that. If you know that users of 32-bit sparc really have a great need for a Fortran compiler, then feel free to investigate further, but I'm not sure how many other 32-bit platforms have an interest in flang/mlir, so maintaining it might turn out to be a non-trivial amount of work.

Otherwise, thanks for the test, this LGTM now. However I'm not familiar with the SPARC ABI - do you know anyone that usually reviews SPARC patches and could have a look at this before it gets committed?



================
Comment at: flang/test/Fir/target-rewrite-complex.fir:5
 // RUN: fir-opt --target-rewrite="target=powerpc64le-unknown-linux-gnu" %s | FileCheck %s --check-prefix=PPC
+// RUN: fir-opt --target-rewrite="target=sparcv9-sun-solaris2.11" %s | FileCheck %s --check-prefix=SPARCV9
 
----------------
Nit: It would be nice to also test a Linux triple here (it can use the same check prefix as Solaris).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133561