[flang-commits] [PATCH] D128260: [Fortran] Ignore whitespace in FCVS test results

[Kiran Chandramohan via Phabricator via flang-commits]

kiranchandramohan added a comment.

The patch application failed. 
Do we need to update the README file with the change?

In D128260#3604080 <https://reviews.llvm.org/D128260#3604080>, @rovka wrote:

> In D128260#3604020 <https://reviews.llvm.org/D128260#3604020>, @kiranchandramohan wrote:
>
>> @rovka Would you know what is the scope of the llvm-testsuite? Is it limited to flang, or is it flang+gfortran, or is it any fortran compiler? And is it possible to have multiple outputs for comparison as @clementval mentioned?
>
> It's not limited to a specific compiler, it "should work" with any compiler. We're currently running it on one of the buildbots with `flang-to-external-fc`, and I'd like to switch it to use `flang-new` instead. However, I think it's valuable to be able to run with both flang and gfortran, so we can easily compare/debug things.
> At the moment I'm not aware of a way to add multiple outputs for comparison, I don't see that in any of the suites included (but I could be missing something).

Because there are a lot of things that are not standardised in Fortran, we might keep running into these issues. Since ifx/ifort also differs from flang/gfortran, the goal of working with any compiler is probably difficult to achieve in Fortran I think.

In D128260#3604620 <https://reviews.llvm.org/D128260#3604620>, @awarzynski wrote:

> In D128260#3604020 <https://reviews.llvm.org/D128260#3604020>, @kiranchandramohan wrote:
>
>> And is it possible to have multiple outputs for comparison as @clementval mentioned?
>
> I'm not too fond of the idea - that would require more bookkeeping and code to maintain. My preference would be to avoid this. Unless llvm-test-suite already has some logic for this (I'm not aware of such).

Yes, it might not be worth the trouble if there is no existing support for this. Otherwise, the changes will be localised to the reference file.



================
Comment at: cmake/modules/SingleMultiSource.cmake:108-117
     if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${ENDIAN}-endian.${SIZE_SUFFIX})
       set(REFERENCE_OUTPUT ${name}.reference_output.${ENDIAN}-endian.${SIZE_SUFFIX})
     elseif(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${SIZE_SUFFIX})
       set(REFERENCE_OUTPUT ${name}.reference_output.${SIZE_SUFFIX})
     elseif(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${ENDIAN}-endian)
       set(REFERENCE_OUTPUT ${name}.reference_output.${ENDIAN}-endian)
     elseif(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.reference_output.${TARGET_OS})
----------------
I am not pushing for this option. But this bit here seems to allow for reference output based on TARGET, ENDIAN-NESS etc.


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

https://reviews.llvm.org/D128260