Apply kind code check on exitstat and cmdstat #78286

yiwu0b11 · 2024-01-16T14:15:33Z

When testing on gcc, both exitstat and cmdstat must be a kind=4 integer, e.g. DefaultInt. This patch changes the input arg requirement from AnyInt to TypePattern{IntType, KindCode::greaterOrEqualToKind, n}.

The standard stated in 16.9.73

EXITSTAT (optional) shall be a scalar of type integer with a decimal exponent range of at least nine.
CMDSTAT (optional) shall be a scalar of type integer with a decimal exponent range of at least four.

program bug
  implicit none
  integer(kind = 2) :: exitstatvar
  integer(kind = 4) :: cmdstatvar 
  character(len=256) :: msg
  character(len=:), allocatable :: command
  command='echo hello'
  call execute_command_line(command, exitstat=exitstatvar, cmdstat=cmdstatvar)
end program

When testing the above program with exitstatvar kind<4, an error would occur:

$ ../build-release/bin/flang-new test.f90 
error: Semantic errors in test.f90
./test.f90:8:47: error: Actual argument for 'exitstat=' has bad type or kind 'INTEGER(2)'
    call execute_command_line(command, exitstat=exitstatvar)

When testing the above program with exitstatvar kind<2, an error would occur:

$ ../build-release/bin/flang-new test.f90 
error: Semantic errors in test.f90
./test.f90:8:47: error: Actual argument for 'cmdstat=' has bad type or kind 'INTEGER(1)'
    call execute_command_line(command, cmdstat=cmdstatvar)

Test file for this semantics has been added to flang/test/Semantic
Fixes: #77990

llvmbot · 2024-01-16T14:16:03Z

@llvm/pr-subscribers-flang-semantics

Author: Yi Wu (yi-wu-arm)

Changes

When testing on gcc, both exitstat and cmdstat must be a kind=4 integer, e.g. DefaultInt. This patch change the input arg requirement from AnyInt to DefaultInt.

The standard stated in 16.9.73

EXITSTAT (optional) shall be a scalar of type integer with a decimal exponent range of at least nine.
CMDSTAT (optional) shall be a scalar of type integer with a decimal exponent range of at least four.

program bug
  implicit none
  integer(kind = 2) :: exitstatvar
  integer(kind = 4) :: cmdstatvar 
  character(len=256) :: msg
  character(len=:), allocatable :: command
  command='echo hello'
  call execute_command_line(command, exitstat=exitstatvar, cmdstat=cmdstatvar)
end program

When testing the above program with kind != 4, an error would occur:

$ ../build-release/bin/flang-new test.f90 
error: Semantic errors in test.f90
./test.f90:8:47: error: Actual argument for 'exitstat=' has bad type or kind 'INTEGER(2)'
    call execute_command_line(command, exitstat=exitstatvar, cmdstat=cmdstatvar)

Fixes: #77990

Full diff: https://github.com/llvm/llvm-project/pull/78286.diff

1 Files Affected:

(modified) flang/lib/Evaluate/intrinsics.cpp (+2-2)

diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index da6d5970089884c..0b9bdac88a78dcf 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1314,9 +1314,9 @@ static const IntrinsicInterface intrinsicSubroutine[]{
     {"execute_command_line",
         {{"command", DefaultChar, Rank::scalar},
             {"wait", AnyLogical, Rank::scalar, Optionality::optional},
-            {"exitstat", AnyInt, Rank::scalar, Optionality::optional,
+            {"exitstat", DefaultInt, Rank::scalar, Optionality::optional,
                 common::Intent::InOut},
-            {"cmdstat", AnyInt, Rank::scalar, Optionality::optional,
+            {"cmdstat", DefaultInt, Rank::scalar, Optionality::optional,
                 common::Intent::Out},
             {"cmdmsg", DefaultChar, Rank::scalar, Optionality::optional,
                 common::Intent::InOut}},

klausler

Absolutely not. If the standard doesn't restrict the types of these arguments, neither can we.

yiwu0b11 · 2024-01-16T16:04:56Z

Absolutely not. If the standard doesn't restrict the types of these arguments, neither can we.

Should it be Defaulnt for existstat, an int32_t(kind==4) would cover +-10^9, and an int16_t(kind==2) that covers +-10^4 for cmdstat?

klausler · 2024-01-16T16:07:44Z

exitstat can be any kind >= 4. cmdstat can be any kind >= 2.

Now execute_command_line will accept exitstat kind>=4, cmdstat kind >=2

yiwu0b11 · 2024-01-16T16:59:20Z

exitstat can be any kind >= 4. cmdstat can be any kind >= 2.

Added a KindCode::greaterAndEqualTo in intrinsics to allow this kind check.

flang/lib/Evaluate/intrinsics.cpp

kiranchandramohan · 2024-01-16T17:26:00Z

You need a test for this patch.

klausler

Is there another patch that adds integer kind handling for these two arguments to the runtime implementation?

yiwu0b11 · 2024-01-23T16:12:51Z

Is there another patch that adds integer kind handling for these two arguments to the runtime implementation?

Not now, sorry, let me add that right now.

klausler · 2024-01-23T17:06:39Z

flang/runtime/execute.cpp

@@ -122,10 +122,22 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,

  if (exitstat) {
    RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat));
+    auto exitstatKind{exitstat->type().GetCategoryAndKind()->second};
+    if (exitstatKind < 4) {


The runtime code doesn't need to check these descriptors -- that will have been done statically when the intrinsic procedure reference was analyzed!

What has to be done in the runtime is using those descriptors to store the integer result values properly, and I think that the code already does that.

Sorry I misunderstood your comment. removed

yiwu0b11 · 2024-01-23T17:07:42Z

Is there another patch that adds integer kind handling for these two arguments to the runtime implementation?

Thanks for mention the kind code in the runtime! I've added kind code check in runtime and provide test, an incorrect kind code would result in a terminator.Crash "exitstat/cmdstat must have an integer kind greater or equal to 4/2 but have: %d". The tests also test the terminator printout.
kind code checks at runtime are removed.

This reverts commit 4d0d1c6.

yiwu0b11 · 2024-01-29T11:02:37Z

build and pass all test on Windows on Arm MSVC native.

When testing on gcc, both exitstat and cmdstat must be a kind=4 integer, e.g. DefaultInt. This patch changes the input arg requirement from `AnyInt` to `TypePattern{IntType, KindCode::greaterOrEqualToKind, n}`. The standard stated in 16.9.73 - EXITSTAT (optional) shall be a scalar of type integer with a decimal exponent range of at least nine. - CMDSTAT (optional) shall be a scalar of type integer with a decimal exponent range of at least four. ```fortran program bug implicit none integer(kind = 2) :: exitstatvar integer(kind = 4) :: cmdstatvar character(len=256) :: msg character(len=:), allocatable :: command command='echo hello' call execute_command_line(command, exitstat=exitstatvar, cmdstat=cmdstatvar) end program ``` When testing the above program with exitstatvar kind<4, an error would occur: ``` $ ../build-release/bin/flang-new test.f90 error: Semantic errors in test.f90 ./test.f90:8:47: error: Actual argument for 'exitstat=' has bad type or kind 'INTEGER(2)' call execute_command_line(command, exitstat=exitstatvar) ``` When testing the above program with exitstatvar kind<2, an error would occur: ``` $ ../build-release/bin/flang-new test.f90 error: Semantic errors in test.f90 ./test.f90:8:47: error: Actual argument for 'cmdstat=' has bad type or kind 'INTEGER(1)' call execute_command_line(command, cmdstat=cmdstatvar) ``` Test file for this semantics has been added to `flang/test/Semantic` Fixes: llvm#77990 (cherry picked from commit 14a1510)

change exitstat and cmsstat from AnyInt to DefaultInt

Verified

This commit was signed with the committer’s verified signature.

kennytm

GPG key ID: FEF6C8051D0E013C

Verified
Learn about vigilant mode

d56eca5

llvmbot added flang flang:semantics labels Jan 16, 2024

yiwu0b11 requested a review from psteinfeld January 16, 2024 14:15

yiwu0b11 requested a review from klausler January 16, 2024 14:16

klausler requested changes Jan 16, 2024

View reviewed changes

add KindCode::greaterEqualToKind

2741652

Now execute_command_line will accept exitstat kind>=4, cmdstat kind >=2

klausler reviewed Jan 16, 2024

View reviewed changes

flang/lib/Evaluate/intrinsics.cpp Outdated Show resolved Hide resolved

yiwu0b11 changed the title ~~change exitstat and cmsstat from AnyInt to DefaultInt~~ Apply kind code check on exitstat and cmdstat Jan 16, 2024

doc fixes

cea4840

yiwu0b11 added 5 commits January 17, 2024 10:41

add test and rename variable

0b04999

doc and comment fixes

450bc78

add a newline at end of the file

98b3962

Merge remote-tracking branch 'yi/main' into ecl_arg_kind_fix

d291717

format and doc fixes

5ac4b76

yiwu0b11 requested a review from klausler January 23, 2024 10:34

klausler removed their request for review January 23, 2024 16:06

klausler reviewed Jan 23, 2024

View reviewed changes

yiwu0b11 added 2 commits January 23, 2024 16:36

Merge remote-tracking branch 'yi/main' into ecl_arg_kind_fix

18a48ef

add runtime kind code check for ExecuteCommandLine

4d0d1c6

yiwu0b11 force-pushed the ecl_arg_kind_fix branch from e572bff to 4d0d1c6 Compare January 23, 2024 17:03

klausler reviewed Jan 23, 2024

View reviewed changes

klausler approved these changes Jan 23, 2024

View reviewed changes

yiwu0b11 added 2 commits January 25, 2024 16:58

Revert "add runtime kind code check for ExecuteCommandLine"

5d041e6

This reverts commit 4d0d1c6.

Merge remote-tracking branch 'yi/main' into ecl_arg_kind_fix

7ab19bb

yiwu0b11 merged commit 14a1510 into llvm:main Jan 29, 2024

pointhex mentioned this pull request May 7, 2024

getStyleDiagHandler #91314

Closed

aemerson mentioned this pull request May 9, 2024

release/18.x: [AArc64][GlobalISel] Fix legalizer assert for G_INSERT_VECTOR_ELT - manual merge #91672

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply kind code check on exitstat and cmdstat #78286

Apply kind code check on exitstat and cmdstat #78286

yiwu0b11 commented Jan 16, 2024 •

edited

Loading

llvmbot commented Jan 16, 2024

klausler left a comment

yiwu0b11 commented Jan 16, 2024 •

edited

Loading

klausler commented Jan 16, 2024

yiwu0b11 commented Jan 16, 2024

kiranchandramohan commented Jan 16, 2024

klausler left a comment

yiwu0b11 commented Jan 23, 2024

klausler Jan 23, 2024

yiwu0b11 Jan 23, 2024 •

edited

Loading

yiwu0b11 commented Jan 23, 2024 •

edited

Loading

yiwu0b11 commented Jan 29, 2024

Apply kind code check on exitstat and cmdstat #78286

Apply kind code check on exitstat and cmdstat #78286

Conversation

yiwu0b11 commented Jan 16, 2024 • edited Loading

llvmbot commented Jan 16, 2024

klausler left a comment

Choose a reason for hiding this comment

yiwu0b11 commented Jan 16, 2024 • edited Loading

klausler commented Jan 16, 2024

yiwu0b11 commented Jan 16, 2024

kiranchandramohan commented Jan 16, 2024

klausler left a comment

Choose a reason for hiding this comment

yiwu0b11 commented Jan 23, 2024

klausler Jan 23, 2024

Choose a reason for hiding this comment

yiwu0b11 Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

yiwu0b11 commented Jan 23, 2024 • edited Loading

yiwu0b11 commented Jan 29, 2024

yiwu0b11 commented Jan 16, 2024 •

edited

Loading

yiwu0b11 commented Jan 16, 2024 •

edited

Loading

yiwu0b11 Jan 23, 2024 •

edited

Loading

yiwu0b11 commented Jan 23, 2024 •

edited

Loading