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

[Rainer Orth via Phabricator via flang-commits]

ro marked an inline comment as done.
ro added a comment.

In D133561#3817542 <https://reviews.llvm.org/D133561#3817542>, @rovka wrote:

> 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

[...]

> 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.

I don't think there will be great need for 32-bit support, so I'll leave this 64-bit only for now.  Maybe others will have the inclination to dive further and sparc can benefit ;-)

> 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?

I've added some of the usual suspects as reviewers.  However, given that SPARC lacks maintainers in LLVM, that's always been difficult.



================
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
 
----------------
rovka wrote:
> Nit: It would be nice to also test a Linux triple here (it can use the same check prefix as Solaris).
Will do.  It should be helpful because Solaris and Linux differ in their `long double` type, although LLVM is pretty broken in that respect right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133561