Re: [PATCH v3] rs6000: Fix some issues in rs6000_can_inline_p [PR102059]
Hi!
On Wed, Jan 05, 2022 at 03:34:41PM +0800, Kewen.Lin wrote:
> This patch is to fix the inconsistent behaviors for non-LTO mode
> and LTO mode. As Martin pointed out, currently the function
> rs6000_can_inline_p simply makes it inlinable if callee_tree is
> NULL, but it's unexpected, we should use the command line options
> from target_option_default_node as default.
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -25379,55 +25379,87 @@ rs6000_update_ipa_fn_target_info (unsigned int
> &info, const gimple *stmt)
> static bool
> rs6000_can_inline_p (tree caller, tree callee)
> {
> - bool ret = false;
>tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
>tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
>
> - /* If the callee has no option attributes, then it is ok to inline. */
> + /* If the caller/callee has option attributes, then use them.
> + Otherwise, use the command line options. */
>if (!callee_tree)
> -ret = true;
> -
> - else
> -{
> - HOST_WIDE_INT caller_isa;
> - struct cl_target_option *callee_opts = TREE_TARGET_OPTION
> (callee_tree);
> - HOST_WIDE_INT callee_isa = callee_opts->x_rs6000_isa_flags;
> - HOST_WIDE_INT explicit_isa = callee_opts->x_rs6000_isa_flags_explicit;
> +callee_tree = target_option_default_node;
> + if (!caller_tree)
> +caller_tree = target_option_default_node;
> +
> + struct cl_target_option *caller_opts = TREE_TARGET_OPTION (caller_tree);
> + struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);
> + HOST_WIDE_INT caller_isa = caller_opts->x_rs6000_isa_flags;
> + HOST_WIDE_INT callee_isa = callee_opts->x_rs6000_isa_flags;
> +
> + bool always_inline
> += DECL_DISREGARD_INLINE_LIMITS (callee)
> + && lookup_attribute ("always_inline", DECL_ATTRIBUTES (callee));
> +
> + /* Some features can be tolerated for always inlines. */
> + unsigned HOST_WIDE_INT always_inline_safe_mask
> +/* Fusion option masks. */
> += OPTION_MASK_P8_FUSION | OPTION_MASK_P10_FUSION
> + | OPTION_MASK_P8_FUSION_SIGN | OPTION_MASK_P10_FUSION
> + | OPTION_MASK_P10_FUSION_LD_CMPI | OPTION_MASK_P10_FUSION_2LOGICAL
> + | OPTION_MASK_P10_FUSION_LOGADD | OPTION_MASK_P10_FUSION_ADDLOG
> + | OPTION_MASK_P10_FUSION_2ADD
> + /* Like fusion, some option masks which are just for optimization. */
> + | OPTION_MASK_SAVE_TOC_INDIRECT | OPTION_MASK_PCREL_OPT;
Why is this? I would expect only two or three options to be *not* safe!
You have a strange mix of internal options (the FUSION* things) and some
other options that do not change the semantics at all. But this is true
for almost *all* options we have! What would not allow a callee that is
allowed to use some newer ISA's insns into a caller that does not allow
them? Both when it ends up using those insns and when it does not, the
end result is valid. If there is something internal to GCC that causes
ICEs we need to do something about *that*!
> + /* Some features are totally safe for inlining (or always inlines),
> + let's exclude them from the following checkings. */
> + HOST_WIDE_INT safe_mask = always_inline ? always_inline_safe_mask : 0;
> + cgraph_node *callee_node = cgraph_node::get (callee);
> + if (ipa_fn_summaries && ipa_fn_summaries->get (callee_node) != NULL)
> +{
> + unsigned int info = ipa_fn_summaries->get (callee_node)->target_info;
> + if ((info & RS6000_FN_TARGET_INFO_HTM) == 0)
> + safe_mask |= OPTION_MASK_HTM;
> +}
always_inline means *always*. If the compiler cannot do this it should
not, but then error; in all other cases it should do it.
This patch is pretty much equal to not allowing any inlining if the
caller and callee have not identical compilation options. So sure, it
causes us to not have any unwanted inlining, but it does that by
preventing all other inlining at the same time.
Segher
Re: [PATCH 1/4][RFC] middle-end/90348 - add explicit birth
On Sat, 5 Feb 2022, Eric Botcazou wrote: > > In the past stack sharing has been quite important for the linux > > kernel. So perhaps one of the tests we should do if we wanted to go > > forward in this cycle would be to test kernel builds to see if any start > > tripping over the stack space diagnostics they've put in place over the > > years. > > Stack slot sharing is important generally speaking, not just for the kernel. > As demonstrated by the recent controversy about the 1/X optimization, last- > minute broad changes made during stage #4 are generally not a good idea, and > I'm not sure what the incentive for fixing PR middle-end/90348 now is (it's > labeled a 9/10/11/12 regression). At a minimum, I think that a switch to > revert to the pre-12 behavior would be in order if the patch goes in now. I don't think an option to go to pre-12 behavior is useful. I'll postpone the series to stage1. Richard.
[PATCH] Check always_inline flag in s390_can_inline_p [PR104327]
MASK_MVCLE is set for -Os but not for other optimization levels. In
general it should not make much sense to inline across calls where the
flag is different but we have to allow it for always_inline.
The patch also rearranges the hook implementation a bit based on the
recommendations from Jakub und Martin in the PR.
Bootstrapped and regression tested on s390x with various arch flags.
Will commit after giving a few days for comments.
gcc/ChangeLog:
PR target/104327
* config/s390/s390.cc (s390_can_inline_p): Accept a few more flags
if always_inline is set. Don't inline when tune differs without
always_inline.
gcc/testsuite/ChangeLog:
PR target/104327
* gcc.c-torture/compile/pr104327.c: New test.
---
gcc/config/s390/s390.cc | 66 ++-
.../gcc.c-torture/compile/pr104327.c | 15 +
2 files changed, 64 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr104327.c
diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
index 5c2a830f9f0..bbf2dd8dfb4 100644
--- a/gcc/config/s390/s390.cc
+++ b/gcc/config/s390/s390.cc
@@ -16091,6 +16091,25 @@ s390_valid_target_attribute_p (tree fndecl,
static bool
s390_can_inline_p (tree caller, tree callee)
{
+ unsigned HOST_WIDE_INT all_masks =
+(MASK_64BIT | MASK_BACKCHAIN | MASK_DEBUG_ARG | MASK_ZARCH
+ | MASK_HARD_DFP | MASK_SOFT_FLOAT
+ | MASK_OPT_HTM | MASK_LONG_DOUBLE_128 | MASK_MVCLE | MASK_PACKED_STACK
+ | MASK_SMALL_EXEC | MASK_OPT_VX | MASK_ZVECTOR);
+
+ /* Flags which if present in the callee are required in the caller as well.
*/
+ unsigned HOST_WIDE_INT caller_required_masks = MASK_OPT_HTM;
+
+ /* Flags which affect the ABI and in general prevent inlining. */
+ unsigned HOST_WIDE_INT must_match_masks =
+(MASK_64BIT | MASK_ZARCH | MASK_HARD_DFP | MASK_SOFT_FLOAT
+ | MASK_LONG_DOUBLE_128 | MASK_OPT_VX);
+
+ /* Flags which we in general want to prevent inlining but accept for
+ always_inline. */
+ unsigned HOST_WIDE_INT always_inline_safe_masks =
+MASK_MVCLE | MASK_BACKCHAIN | MASK_SMALL_EXEC;
+
tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
@@ -16103,16 +16122,18 @@ s390_can_inline_p (tree caller, tree callee)
struct cl_target_option *caller_opts = TREE_TARGET_OPTION (caller_tree);
struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);
- bool ret = true;
- if ((caller_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP))
- != (callee_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP)))
-ret = false;
+ /* If one of these triggers make sure to add proper handling of your
+ new flag to this hook. */
+ gcc_assert (!(caller_opts->x_target_flags & ~all_masks));
+ gcc_assert (!(callee_opts->x_target_flags & ~all_masks));
- /* Don't inline functions to be compiled for a more recent arch into a
- function for an older arch. */
- else if (caller_opts->x_s390_arch < callee_opts->x_s390_arch)
-ret = false;
+ bool always_inline
+= (DECL_DISREGARD_INLINE_LIMITS (callee)
+ && lookup_attribute ("always_inline", DECL_ATTRIBUTES (callee)));
+
+ if (!always_inline)
+must_match_masks |= always_inline_safe_masks;
/* Inlining a hard float function into a soft float function is only
allowed if the hard float function doesn't actually make use of
@@ -16120,16 +16141,27 @@ s390_can_inline_p (tree caller, tree callee)
We are called from FEs for multi-versioning call optimization, so
beware of ipa_fn_summaries not available. */
- else if (((TARGET_SOFT_FLOAT_P (caller_opts->x_target_flags)
-&& !TARGET_SOFT_FLOAT_P (callee_opts->x_target_flags))
- || (!TARGET_HARD_DFP_P (caller_opts->x_target_flags)
- && TARGET_HARD_DFP_P (callee_opts->x_target_flags)))
- && (! ipa_fn_summaries
- || ipa_fn_summaries->get
- (cgraph_node::get (callee))->fp_expressions))
-ret = false;
+ if (always_inline && ipa_fn_summaries
+ && !ipa_fn_summaries->get(cgraph_node::get (callee))->fp_expressions)
+must_match_masks &= ~(MASK_HARD_DFP | MASK_SOFT_FLOAT);
- return ret;
+ if ((caller_opts->x_target_flags & must_match_masks)
+ != (callee_opts->x_target_flags & must_match_masks))
+return false;
+
+ if (~(caller_opts->x_target_flags & caller_required_masks)
+ & (callee_opts->x_target_flags & caller_required_masks))
+return false;
+
+ /* Don't inline functions to be compiled for a more recent arch into a
+ function for an older arch. */
+ if (caller_opts->x_s390_arch < callee_opts->x_s390_arch)
+return false;
+
+ if (!always_inline && caller_opts->x_s390_tune != callee_opts->x_s390_tune)
+return false;
+
+ return true;
}
#endif
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr104327.c
b/gcc/testsuite/gcc.c-torture/co
[PATCH] RISC-V: Enable TARGET_SUPPORTS_WIDE_INT
This is at par with other major arches such as aarch64, i386, s390 ...
No testsuite regressions: same numbers w/ w/o
| === gcc Summary ===
|
|# of expected passes 113392
|# of unexpected failures 27
|# of unexpected successes 3
|# of expected failures 605
|# of unsupported tests 2523
|
| === g++ Summary ===
|
|# of expected passes 172997
|# of unexpected failures 26
|# of expected failures 706
|# of unsupported tests 9566
Signed-off-by: Vineet Gupta
---
gcc/config/riscv/predicates.md | 2 +-
gcc/config/riscv/riscv.c | 6 ++
gcc/config/riscv/riscv.h | 2 ++
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
index 3da6fd4c0491..cf902229954b 100644
--- a/gcc/config/riscv/predicates.md
+++ b/gcc/config/riscv/predicates.md
@@ -52,7 +52,7 @@
(match_test "INTVAL (op) + 1 != 0")))
(define_predicate "const_0_operand"
- (and (match_code "const_int,const_wide_int,const_double,const_vector")
+ (and (match_code "const_int,const_wide_int,const_vector")
(match_test "op == CONST0_RTX (GET_MODE (op))")))
(define_predicate "reg_or_0_operand"
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index c830cd8f4ad1..d2f2d9e0276f 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -1774,6 +1774,12 @@ riscv_rtx_costs (rtx x, machine_mode mode, int
outer_code, int opno ATTRIBUTE_UN
case SYMBOL_REF:
case LABEL_REF:
case CONST_DOUBLE:
+ /* With TARGET_SUPPORTS_WIDE_INT const int can't be in CONST_DOUBLE
+ rtl object. Weird recheck due to switch-case fall through above. */
+ if (GET_CODE (x) == CONST_DOUBLE)
+gcc_assert (GET_MODE (x) != VOIDmode);
+ /* Fall through. */
+
case CONST:
if ((cost = riscv_const_insns (x)) > 0)
{
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index ff6729aedac2..91cfc82b4aa4 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -997,4 +997,6 @@ extern void riscv_remove_unneeded_save_restore_calls (void);
#define HARD_REGNO_RENAME_OK(FROM, TO) riscv_hard_regno_rename_ok (FROM, TO)
+#define TARGET_SUPPORTS_WIDE_INT 1
+
#endif /* ! GCC_RISCV_H */
--
2.32.0
PING^3 [PATCH v3] rs6000: Fix some issues in rs6000_can_inline_p [PR102059]
Gentle ping this: https://gcc.gnu.org/pipermail/gcc-patches/2022-January/587635.html BR, Kewen >> on 2022/1/5 下午3:34, Kewen.Lin via Gcc-patches wrote: >>> Hi, >>> >>> This patch is to fix the inconsistent behaviors for non-LTO mode >>> and LTO mode. As Martin pointed out, currently the function >>> rs6000_can_inline_p simply makes it inlinable if callee_tree is >>> NULL, but it's unexpected, we should use the command line options >>> from target_option_default_node as default. >>> >>> It replaces rs6000_isa_flags with target_option_default_node when >>> caller_tree is NULL since it's more straightforward and doesn't >>> suffer from some bug not to keep rs6000_isa_flags as default. >>> >>> It also extends the scope of the check for the case that callee >>> has explicit set options, inlining in test case pr102059-5.c can >>> happen unexpectedly before, it's fixed accordingly. >>> >>> As Richi/Mike pointed out, some tuning flags like MASK_P8_FUSION >>> can be neglected for always inlining, this patch also takes some >>> flags when the callee is attributed by always_inline. >>> >>> v1: https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578552.html >>> v2: https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586112.html >>> >>> This patch is one re-post of this updated version[1] and also >>> rebased and adjusted on top of the related commit r12-6219. >>> >>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and >>> powerpc64le-linux-gnu P9 and P10. >>> >>> Is it ok for trunk? >>> >>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586296.html >>> >>> BR, >>> Kewen >>> - >>> gcc/ChangeLog: >>> >>> PR target/102059 >>> * config/rs6000/rs6000.c (rs6000_can_inline_p): Adjust with >>> target_option_default_node and consider always_inline_safe flags. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR target/102059 >>> * gcc.target/powerpc/pr102059-4.c: New test. >>> * gcc.target/powerpc/pr102059-5.c: New test. >>> * gcc.target/powerpc/pr102059-6.c: New test. >>> * gcc.target/powerpc/pr102059-7.c: New test. >>> * gcc.target/powerpc/pr102059-8.c: New test. >>> * gcc.dg/lto/pr102059-1_0.c: Remove unneeded option. >>> >>>
Re: [PATCH v2] rs6000: Disable MMA if no VSX support [PR103627]
on 2022/1/28 上午1:17, Segher Boessenkool wrote: > Hi! > > On Thu, Jan 27, 2022 at 07:21:33PM +0800, Kewen.Lin wrote: >> PR target/103627 >> * config/rs6000/rs6000.cc (rs6000_option_override_internal): Disable >> MMA if !TARGET_VSX. >> >> gcc/testsuite/ChangeLog: >> >> PR target/103627 >> * gcc.target/powerpc/pr103627-1.c: New test. >> * gcc.target/powerpc/pr103627-2.c: New test. > > Okay for trunk. Thanks! > > Thanks Segher! Commit it as r12-7078 and its successor as r12-7079. Since it's related to MMA support, I guess we want to backport it? If so, is it ok to backport to GCC 10 and 11 after one week or so? BR, Kewen
Re: [PATCH] RISC-V: Add target machine headers as a dependency for riscv-sr.o
OK to trunk, thanks for fixing this issue, I hit that issue before but I didn't figure out what happened...since that issue will disappear when I clean build :p On Tue, Feb 1, 2022 at 5:52 AM Maciej W. Rozycki wrote: > > Make riscv-sr.o depend on target machine headers, removing spurious test > failures: > > FAIL: gcc.target/riscv/save-restore-3.c scan-assembler-not call[ > \t]*t0,__riscv_save_0 > FAIL: gcc.target/riscv/save-restore-3.c scan-assembler-not tail[ > \t]*__riscv_restore_0 > FAIL: gcc.target/riscv/save-restore-3.c scan-assembler tail[ \t]*foo > FAIL: gcc.target/riscv/save-restore-6.c scan-assembler-not call[ > \t]*t0,__riscv_save_0 > FAIL: gcc.target/riscv/save-restore-6.c scan-assembler-not tail[ > \t]*__riscv_restore_0 > FAIL: gcc.target/riscv/save-restore-6.c scan-assembler tail[ \t]*other_func > > if the definitions of UNSPECs are locally changed and GCC rebuilt from a > dirty tree. > > gcc/ > * config/riscv/t-riscv (riscv-sr.o): Add $(TM_H) dependency. > --- > Hi, > > Noticed while fiddling with `fmin'/`fmax' further. As not a code change > OK to apply despite stage 4? Might be worth backporting too. > > Maciej > --- > gcc/config/riscv/t-riscv |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > gcc-riscv-sr-dep.diff > Index: gcc/gcc/config/riscv/t-riscv > === > --- gcc.orig/gcc/config/riscv/t-riscv > +++ gcc/gcc/config/riscv/t-riscv > @@ -6,7 +6,7 @@ riscv-builtins.o: $(srcdir)/config/riscv > $(srcdir)/config/riscv/riscv-builtins.cc > > riscv-sr.o: $(srcdir)/config/riscv/riscv-sr.cc $(CONFIG_H) \ > - $(SYSTEM_H) > + $(SYSTEM_H) $(TM_H) > $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \ > $(srcdir)/config/riscv/riscv-sr.cc >
[PATCH] PR fortran/66193 - ICE for initialisation of some non-zero-sized arrays
Dear Fortranners,
some instances of valid constant array constructors did lead to ICEs.
It turned out that on the one hand we need to attempt simplification of
elements of the constructor, especially when we encounter parenthesized
expression. On the other hand the occurence of type specs and empty
constructors need to be handled more gracefully.
Parts of the PR have been fixed previously, so the remaining part was
rather simple.
The testcase is based on Gerhards latest example attached to the PR.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Given the simplicity of the patch and that it is an ICE on valid code,
would this qualify for later application to 11-branch?
Thanks,
Harald
From 65966b608cff757f43316e1aeed37bd07ce63fe2 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Sun, 6 Feb 2022 21:47:20 +0100
Subject: [PATCH] Fortran: try simplifications during reductions of array
constructors
gcc/fortran/ChangeLog:
PR fortran/66193
* arith.cc (reduce_binary_ac): When reducing binary expressions,
try simplification. Handle case of empty constructor.
(reduce_binary_ca): Likewise.
gcc/testsuite/ChangeLog:
PR fortran/66193
* gfortran.dg/array_constructor_55.f90: New test.
---
gcc/fortran/arith.cc | 36 ++--
.../gfortran.dg/array_constructor_55.f90 | 55 +++
2 files changed, 85 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/array_constructor_55.f90
diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index b3323ecf640..06e032e22db 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -1305,6 +1305,8 @@ reduce_binary_ac (arith (*eval) (gfc_expr *, gfc_expr *, gfc_expr **),
head = gfc_constructor_copy (op1->value.constructor);
for (c = gfc_constructor_first (head); c; c = gfc_constructor_next (c))
{
+ gfc_simplify_expr (c->expr, 0);
+
if (c->expr->expr_type == EXPR_CONSTANT)
rc = eval (c->expr, op2, &r);
else
@@ -1321,9 +1323,19 @@ reduce_binary_ac (arith (*eval) (gfc_expr *, gfc_expr *, gfc_expr **),
else
{
gfc_constructor *c = gfc_constructor_first (head);
- r = gfc_get_array_expr (c->expr->ts.type, c->expr->ts.kind,
- &op1->where);
- r->shape = gfc_copy_shape (op1->shape, op1->rank);
+ if (c)
+ {
+ r = gfc_get_array_expr (c->expr->ts.type, c->expr->ts.kind,
+ &op1->where);
+ r->shape = gfc_copy_shape (op1->shape, op1->rank);
+ }
+ else
+ {
+ gcc_assert (op1->ts.type != BT_UNKNOWN);
+ r = gfc_get_array_expr (op1->ts.type, op1->ts.kind,
+ &op1->where);
+ r->shape = gfc_get_shape (op1->rank);
+ }
r->rank = op1->rank;
r->value.constructor = head;
*result = r;
@@ -1345,6 +1357,8 @@ reduce_binary_ca (arith (*eval) (gfc_expr *, gfc_expr *, gfc_expr **),
head = gfc_constructor_copy (op2->value.constructor);
for (c = gfc_constructor_first (head); c; c = gfc_constructor_next (c))
{
+ gfc_simplify_expr (c->expr, 0);
+
if (c->expr->expr_type == EXPR_CONSTANT)
rc = eval (op1, c->expr, &r);
else
@@ -1361,9 +1375,19 @@ reduce_binary_ca (arith (*eval) (gfc_expr *, gfc_expr *, gfc_expr **),
else
{
gfc_constructor *c = gfc_constructor_first (head);
- r = gfc_get_array_expr (c->expr->ts.type, c->expr->ts.kind,
- &op2->where);
- r->shape = gfc_copy_shape (op2->shape, op2->rank);
+ if (c)
+ {
+ r = gfc_get_array_expr (c->expr->ts.type, c->expr->ts.kind,
+ &op2->where);
+ r->shape = gfc_copy_shape (op2->shape, op2->rank);
+ }
+ else
+ {
+ gcc_assert (op2->ts.type != BT_UNKNOWN);
+ r = gfc_get_array_expr (op2->ts.type, op2->ts.kind,
+ &op2->where);
+ r->shape = gfc_get_shape (op2->rank);
+ }
r->rank = op2->rank;
r->value.constructor = head;
*result = r;
diff --git a/gcc/testsuite/gfortran.dg/array_constructor_55.f90 b/gcc/testsuite/gfortran.dg/array_constructor_55.f90
new file mode 100644
index 000..52142cb10c0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_constructor_55.f90
@@ -0,0 +1,55 @@
+! { dg-do run }
+! PR fortran/66193 - ICE for initialisation of some non-zero-sized arrays
+! Testcase by G.Steinmetz
+
+program p
+ implicit none
+ call s1
+ call s2
+ call s3
+ call s4
+contains
+ subroutine s1
+integer(8), parameter :: z1(2) = 10 + [ integer(8) :: [ integer(4) ::],1,2]
+integer(8):: z2(2) = 10 + [ integer(8) :: [ integer(4) ::],1,2]
+integer(8):: z3(2)
+z3 = 10 + [ integer(8) :: [ integer(4) :: ], 1, 2 ]
+if ( z1(1) /= 11 .or. z1(2) /= 12 ) stop 1
+if ( z2(1) /= 11 .or. z2(2) /= 12 ) stop 2
+if ( z3(1) /= 11 .or. z3(2) /= 12 ) stop 3
+ end subroutine s1
+
+ subroutine s2
+logical(8), parameter :: z1(3) = .true. .or. &
+ [ logical(8) :: [ logical(4) :: ], .false., .false., .true. ]
+logical(8):: z2(3) = .true. .or. &
+ [ logical(8) :: [ logical(4) :: ], .false.,
Re: [PATCH] PR/101135 - Load of null pointer when passing absent assumed-shape array argument for an optional dummy argument
Hi Mikael, Am 04.02.22 um 11:45 schrieb Mikael Morin: Hello, Le 29/01/2022 à 22:41, Harald Anlauf via Fortran a écrit : The least invasive change - already pointed out by the reporter - is to check the presence of the argument before dereferencing the data pointer after the offset calculation. This requires adjusting the checking pattern for gfortran.dg/missing_optional_dummy_6a.f90. Regtesting reminded me that procedures with bind(c) attribute are doing their own stuff, which is why they need to be excluded here, otherwise testcase bind-c-contiguous-4.f90 would regress on the expected output. only after submitting the patch I figured that the patch is incomplete. When we have a call chain of procedures with and without bind(c), there are still cases left where the failure with the sanitizer is not fixed. Just add "bind(c)" to subroutine test_wrapper only in the original PR. I have added a corresponding comment in the PR. There is a potential alternative solution which I did not pursue, as I think it is more invasive, but also that I didn't succeed to implement: A non-present dummy array argument should not need to get its descriptor set up. Pursuing this is probably not the right thing to do during the current stage of development and could be implemented later. If somebody believes this is important, feel free to open a PR for this. I have an other (equally unimportant) concern that it may create an unnecessary conditional when passing a subobject of an optional argument. In that case we can assume that the optional is present. It’s not a correctness issue, so let’s not bother at this stage. Judging from the dump tree of the cases I looked at I did not see anything that would pose a problem to the optimizer. Regtested on x86_64-pc-linux-gnu. OK for mainline? OK. Given my latest observations I'd rather withdraw the current version of the patch and rethink. I also did not see an issue with bind(c) procedures calling alikes. It would help if one would not only know the properties of the actual argument, but also of the formal one, which is not available at that point in the code. I'll have another look and resubmit. Thanks. Thanks for the review! Harald
