Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[flang] Place MIN/MAX A1/A2 first in semantic analysis #69722

Merged
merged 2 commits into from
Oct 23, 2023
Merged

[flang] Place MIN/MAX A1/A2 first in semantic analysis #69722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e508a6e
[flang] Place MIN/MAX A1/A2 first in semantic analysis
jeanPerier Oct 20, 2023
8a08e0e
address comment: remove wasInserted
jeanPerier Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions flang/include/flang/Evaluate/intrinsics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ class IntrinsicProcTable {
// On success, the actual arguments are transferred to the result
// in dummy argument order; on failure, the actual arguments remain
// untouched.
// For MIN and MAX, only a1 and a2 actual arguments are transferred in dummy
// order on success and the other arguments are transferred afterwards
// without being sorted.
std::optional<SpecificCall> Probe(
const CallCharacteristics &, ActualArguments &, FoldingContext &) const;

Expand Down
134 changes: 62 additions & 72 deletions flang/lib/Evaluate/intrinsics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1446,63 +1446,75 @@ static std::int64_t GetBuiltinKind(

// Ensure that the keywords of arguments to MAX/MIN and their variants
// are of the form A123 with no duplicates or leading zeroes.
static bool CheckMaxMinArgument(std::optional<parser::CharBlock> keyword,
static bool CheckMaxMinArgument(parser::CharBlock keyword,
std::set<parser::CharBlock> &set, const char *intrinsicName,
parser::ContextualMessages &messages) {
if (keyword) {
std::size_t j{1};
for (; j < keyword->size(); ++j) {
char ch{(*keyword)[j]};
if (ch < (j == 1 ? '1' : '0') || ch > '9') {
break;
}
std::size_t j{1};
for (; j < keyword.size(); ++j) {
char ch{(keyword)[j]};
if (ch < (j == 1 ? '1' : '0') || ch > '9') {
break;
}
if (keyword->size() < 2 || (*keyword)[0] != 'a' || j < keyword->size()) {
messages.Say(*keyword,
"Argument keyword '%s=' is not known in call to '%s'"_err_en_US,
*keyword, intrinsicName);
}
if (keyword.size() < 2 || (keyword)[0] != 'a' || j < keyword.size()) {
messages.Say(keyword,
"argument keyword '%s=' is not known in call to '%s'"_err_en_US,
keyword, intrinsicName);
return false;
}
if (!set.insert(keyword).second) {
messages.Say(keyword,
"argument keyword '%s=' was repeated in call to '%s'"_err_en_US,
keyword, intrinsicName);
return false;
}
return true;
}

// Validate the keyword, if any, and ensure that A1 and A2 are always placed in
// first and second position in actualForDummy. A1 and A2 are special since they
// are not optional. The rest of the arguments are not sorted, there are no
// differences between them.
static bool CheckAndPushMinMaxArgument(ActualArgument &arg,
std::vector<ActualArgument *> &actualForDummy,
std::set<parser::CharBlock> &set, const char *intrinsicName,
parser::ContextualMessages &messages) {
if (std::optional<parser::CharBlock> keyword{arg.keyword()}) {
if (!CheckMaxMinArgument(*keyword, set, intrinsicName, messages)) {
return false;
}
auto [_, wasInserted]{set.insert(*keyword)};
if (!wasInserted) {
const bool isA1{*keyword == parser::CharBlock{"a1", 2}};
if (isA1 && !actualForDummy[0]) {
actualForDummy[0] = &arg;
return true;
}
const bool isA2{*keyword == parser::CharBlock{"a2", 2}};
if (isA2 && !actualForDummy[1]) {
actualForDummy[1] = &arg;
return true;
}
if (isA1 || isA2) {
// Note that for arguments other than a1 and a2, this error will be caught
// later in check-call.cpp.
messages.Say(*keyword,
"Argument keyword '%s=' was repeated in call to '%s'"_err_en_US,
"keyword argument '%s=' to intrinsic '%s' was supplied "
"positionally by an earlier actual argument"_err_en_US,
*keyword, intrinsicName);
return false;
}
}
return true;
}

static void CheckMaxMinA1A2Argument(const ActualArguments &arguments,
std::set<parser::CharBlock> &set, parser::ContextualMessages &messages) {
parser::CharBlock kwA1{"a1", 2};
parser::CharBlock kwA2{"a2", 2};
bool missingA1{set.find(kwA1) == set.end()};
bool missingA2{set.find(kwA2) == set.end()};

if (arguments.size() > 1) {
if (arguments.at(0)->keyword()) {
// If the keyword is specified in the first argument, the following
// arguments must have the keywords.
if (missingA1 && missingA2) {
messages.Say("missing mandatory '%s=' and '%s=' arguments"_err_en_US,
kwA1.ToString(), kwA2.ToString());
} else if (missingA1 && !missingA2) {
messages.Say(
"missing mandatory '%s=' argument"_err_en_US, kwA1.ToString());
} else if (!missingA1 && missingA2) {
messages.Say(
"missing mandatory '%s=' argument"_err_en_US, kwA2.ToString());
}
} else if (arguments.at(1)->keyword()) {
// No keyword is specified in the first argument.
if (missingA1 && missingA2) {
messages.Say(
"missing mandatory '%s=' argument"_err_en_US, kwA2.ToString());
} else {
if (actualForDummy.size() == 2) {
if (!actualForDummy[0] && !actualForDummy[1]) {
actualForDummy[0] = &arg;
return true;
} else if (!actualForDummy[1]) {
actualForDummy[1] = &arg;
return true;
}
}
}
actualForDummy.push_back(&arg);
return true;
}

static bool CheckAtomicKind(const ActualArgument &arg,
Expand Down Expand Up @@ -1552,7 +1564,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
bool isMaxMin{dummyArgPatterns > 0 &&
dummy[dummyArgPatterns - 1].optionality == Optionality::repeats};
std::vector<ActualArgument *> actualForDummy(
isMaxMin ? 0 : dummyArgPatterns, nullptr);
isMaxMin ? 2 : dummyArgPatterns, nullptr);
bool anyMissingActualArgument{false};
std::set<parser::CharBlock> maxMinKeywords;
bool anyKeyword{false};
Expand All @@ -1579,9 +1591,8 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
continue;
}
if (isMaxMin) {
if (CheckMaxMinArgument(arg->keyword(), maxMinKeywords, name, messages)) {
actualForDummy.push_back(&*arg);
} else {
if (!CheckAndPushMinMaxArgument(
*arg, actualForDummy, maxMinKeywords, name, messages)) {
return std::nullopt;
}
} else {
Expand Down Expand Up @@ -1627,17 +1638,6 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
}
}

if (isMaxMin) {
int nArgs{0};
// max() / max(x) is invalid
while ((arguments.size() + nArgs) < 2) {
actualForDummy.push_back(nullptr);
nArgs++;
}

CheckMaxMinA1A2Argument(arguments, maxMinKeywords, messages);
}

std::size_t dummies{actualForDummy.size()};

// Check types and kinds of the actual arguments against the intrinsic's
Expand All @@ -1659,18 +1659,8 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
if (!arg) {
if (d.optionality == Optionality::required) {
std::string kw{d.keyword};
if (isMaxMin && maxMinKeywords.size() == 1) {
// max(a1=x) or max(a2=x)
const auto kwA1{dummy[0].keyword};
const auto kwA2{dummy[1].keyword};
if (maxMinKeywords.begin()->ToString() == kwA1) {
messages.Say("missing mandatory 'a2=' argument"_err_en_US);
} else if (maxMinKeywords.begin()->ToString() == kwA2) {
messages.Say("missing mandatory 'a1=' argument"_err_en_US);
} else {
messages.Say(
"missing mandatory 'a1=' and 'a2=' arguments"_err_en_US);
}
if (isMaxMin && !actualForDummy[0] && !actualForDummy[1]) {
messages.Say("missing mandatory 'a1=' and 'a2=' arguments"_err_en_US);
} else {
messages.Say(
"missing mandatory '%s=' argument"_err_en_US, kw.c_str());
Expand Down
34 changes: 34 additions & 0 deletions flang/test/Lower/Intrinsics/min.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
! RUN: bbc -emit-hlfir -o - %s 2>&1 | FileCheck %s
! Test that min/max A(X>2) optional arguments are handled regardless
! of the order in which they appear. Only A1 and A2 are mandatory.

real function test(a, b, c)
real, optional :: a, b, c
test = min(a1=a, a3=c, a2=c)
end function
! CHECK-LABEL: func.func @_QPtest(
! CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<f32> {fir.bindc_name = "a", fir.optional},
! CHECK-SAME: %[[VAL_1:.*]]: !fir.ref<f32> {fir.bindc_name = "b", fir.optional},
! CHECK-SAME: %[[VAL_2:.*]]: !fir.ref<f32> {fir.bindc_name = "c", fir.optional}) -> f32 {
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_0]] {{.*}}uniq_name = "_QFtestEa"}
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_1]] {{.*}}uniq_name = "_QFtestEb"}
! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_2]] {{.*}}uniq_name = "_QFtestEc"}
! CHECK: %[[VAL_6:.*]] = fir.alloca f32 {bindc_name = "test", uniq_name = "_QFtestEtest"}
! CHECK: %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_6]] {uniq_name = "_QFtestEtest"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
! CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<f32>
! CHECK: %[[VAL_9:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<f32>
! CHECK: %[[VAL_10:.*]] = fir.is_present %[[VAL_5]]#0 : (!fir.ref<f32>) -> i1
! CHECK: %[[VAL_11:.*]] = arith.cmpf olt, %[[VAL_8]], %[[VAL_9]] : f32
! CHECK: %[[VAL_12:.*]] = arith.select %[[VAL_11]], %[[VAL_8]], %[[VAL_9]] : f32
! CHECK: %[[VAL_13:.*]] = fir.if %[[VAL_10]] -> (f32) {
! CHECK: %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<f32>
! CHECK: %[[VAL_15:.*]] = arith.cmpf olt, %[[VAL_12]], %[[VAL_14]] : f32
! CHECK: %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_12]], %[[VAL_14]] : f32
! CHECK: fir.result %[[VAL_16]] : f32
! CHECK: } else {
! CHECK: fir.result %[[VAL_12]] : f32
! CHECK: }
! CHECK: hlfir.assign %[[VAL_13]] to %[[VAL_7]]#0 : f32, !fir.ref<f32>
! CHECK: %[[VAL_17:.*]] = fir.load %[[VAL_7]]#1 : !fir.ref<f32>
! CHECK: return %[[VAL_17]] : f32
! CHECK: }
22 changes: 15 additions & 7 deletions flang/test/Semantics/call23.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
! RUN: %python %S/test_errors.py %s %flang_fc1
! Check errors on MAX/MIN with keywords, a weird case in Fortran
real :: x = 0.0, y = 0.0 , y1 = 0.0 ! prevent folding
!ERROR: Argument keyword 'a1=' was repeated in call to 'max'
!ERROR: argument keyword 'a1=' was repeated in call to 'max'
print *, max(a1=x,a1=1)
!ERROR: Keyword argument 'a1=' has already been specified positionally (#1) in this procedure reference
!ERROR: keyword argument 'a1=' to intrinsic 'max' was supplied positionally by an earlier actual argument
print *, max(x,a1=1)
!ERROR: Keyword argument 'a1=' has already been specified positionally (#1) in this procedure reference
!ERROR: keyword argument 'a1=' to intrinsic 'min1' was supplied positionally by an earlier actual argument
print *, min1(1.,a1=2.,a2=3.)
print *, max(a1=x,a2=0,a4=0) ! ok
print *, max(x,0,a99=0) ! ok
!ERROR: Argument keyword 'a06=' is not known in call to 'max'
!ERROR: argument keyword 'a06=' is not known in call to 'max'
print *, max(a1=x,a2=0,a06=0)
!ERROR: missing mandatory 'a2=' argument
print *, max(a3=y, a1=x)
Expand All @@ -29,10 +29,18 @@
print *, max(a2=y)
!ERROR: missing mandatory 'a1=' and 'a2=' arguments
print *, max(a3=x)
!ERROR: Argument keyword 'a0=' is not known in call to 'max'
!ERROR: argument keyword 'a0=' is not known in call to 'max'
print *, max(a0=x)
!ERROR: Argument keyword 'a03=' is not known in call to 'max'
!ERROR: argument keyword 'a03=' is not known in call to 'max'
print *, max(a03=x)
!ERROR: Argument keyword 'abad=' is not known in call to 'max'
!ERROR: argument keyword 'abad=' is not known in call to 'max'
print *, max(abad=x)
!ERROR: actual argument #2 without a keyword may not follow an actual argument with a keyword
print *, max(a1=x, y)
!ERROR: Keyword argument 'a3=' has already been specified positionally (#3) in this procedure reference
print *, max(x, y, y1, a3=x)
print *, max(a3=x, a4=x, a2=x, a1=x) ! ok
print *, max(a3=x, a2=y, a4=x, a1=x) ! ok
print *, max(x, a2=y, a5=x, a4=x) ! ok
print *, max(x, y, a4=x, a6=x) ! ok
end