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

[Diana Picus via Phabricator via llvm-commits]

rovka added a comment.

In D128260#3604911 <https://reviews.llvm.org/D128260#3604911>, @kiranchandramohan wrote:

> The patch application failed.

Right, not sure why it doesn't find those files, I'll try a few things...

> Do we need to update the README file with the change?

I don't think so, since this doesn't affect the FCVS files themselves.

> 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})
----------------
kiranchandramohan wrote:
> I am not pushing for this option. But this bit here seems to allow for reference output based on TARGET, ENDIAN-NESS etc.
Right, I guess we could add some logic here based on COMPILER_ID or something of the sort. 


Repository:
  rT test-suite

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

https://reviews.llvm.org/D128260