[AMD Official Use Only] Hi Andrew, Request to review the attached patch file and please find comments inline below ... Regards, Rupesh P > -----Original Message----- > From: Andrew Burgess > Sent: Saturday, January 22, 2022 12:37 AM > > Compiler Version: > > gcc (GCC) 12.0.0 20211122 (experimental) > > I guess this must be something that was fixed recently, I tried with the 9.3 > compiler I have locally and the tests failed. I also tried with some random > build from git from early last year, and that also failed. > > Ideally the tests wont just fail when using older tools, but instead the test will > detect that my compiler isn't good enough and just skip the tests, so you > might need to try with some older compilers. > The complete support for assumed rank is recently added in gcc. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103315 . Made changes in the testcase for skipping the testcase run for older compilers. > > > From 383c561a1e926b7b65a1d8ffb5d70a047a4d4559 Mon Sep 17 00:00:00 > 2001 > > From: rupesh > > Date: Fri, 29 Oct 2021 11:32:58 +0530 > > Subject: [PATCH] Support for Fortran's ASSUMED RANK. > > > > gdb/ChangeLog: > > > > * dwarf2/loc.c: (dwarf2_locexpr_baton_eval): Push array dimension > onto the stack. > > * dwarf2/loc.h: Added an additional parameter to the function > dwarf2_evaluate_property. > > * dwarf2/read.c: (scan_partial_symbols): Process > DW_TAG_generic_subrange. > > * (add_partial_symbol): Process DW_TAG_generic_subrange. > > * (process_die): Process DW_TAG_generic_subrange. > > * (is_type_tag_for_partial) : Check for DW_TAG_generic_subrange type. > > * (load_partial_dies): Load DW_TAG_generic_subrange. > > * (new_symbol): Create entry for DW_TAG_generic_subrange type. > > * (read_type_die_1): Read DW_TAG_generic_subrange type. > > * (set_die_type) : Add dynamic property type for DW_AT_rank. > > * f-typeprint.c: (f_type_print_varspec_suffix): Removed > TYPE_DATA_LOCATION. > > * findvar.c: (follow_static_link): Passing new argument to the function > call dwarf2_evaluate_property. > > * gdbtypes.c: (resolve_dynamic_range): Passing new argument to the > function call dwarf2_evaluate_property. > > * (resolve_dynamic_array_or_string): Handle rank dynamic property by > creating and removing types. > > * gdbtypes.h: (DYN_PROP_RANK, TYPE_DYN_PROP, TYPE_RANK_PROP): > New Macros > > * gnu-v3-abi.c: Passing new argument to the function call > dwarf2_evaluate_property. > > * testsuite/gdb.fortran/assumedrank.exp: New Testcase > > * testsuite/gdb.fortran/assumedrank.f90: New Testcase > > ChangeLog entries are no longer needed for GDB, though you are welcome > to have content like this in the commit message if you like. That said, the > preference in GDB is to have the commit described within the main body of > the commit message - a ChangeLog formatted block should be in addition. > > You should still keep the lines under ~76ish characters though. Added commit message describing the main body of the commit message. And also kept lines under ~76ish. > > diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index > > 182f15e7077..60f831c2cff 100644 > > --- a/gdb/dwarf2/loc.c > > +++ b/gdb/dwarf2/loc.c > > @@ -1547,7 +1547,8 @@ dwarf2_locexpr_baton_eval (const struct > dwarf2_locexpr_baton *dlbaton, > > const struct property_addr_info *addr_stack, > > CORE_ADDR *valp, > > bool push_initial_value, > > - bool *is_reference) > > + bool *is_reference, > > + int rank) > > { > > if (dlbaton == NULL || dlbaton->size == 0) > > return 0; > > @@ -1559,6 +1560,10 @@ dwarf2_locexpr_baton_eval (const struct > dwarf2_locexpr_baton *dlbaton, > > value *result; > > scoped_value_mark free_values; > > > > + /* push rank value to the stack */ > > + if (rank) > > + ctx.push_address((rank - 1), false); > > + > > I wonder, did you consider making use of the push_initial_value mechanism? > I considered accommodating changes using push_initial_value mechanism. The push_initial_value is pushing address to the stack the requirement is to push scalar value onto the stack. So I could not make use of this mechanism. > > if (push_initial_value) > > { > > if (addr_stack != nullptr) > > diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp > > b/gdb/testsuite/gdb.fortran/assumedrank.exp > > new file mode 100644 > > index 00000000000..e3961d00278 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp > > @@ -0,0 +1,79 @@ > > +# Copyright 2021 Free Software Foundation, Inc. > > + > > +# This program is free software; you can redistribute it and/or > > +modify # it under the terms of the GNU General Public License as > > +published by # the Free Software Foundation; either version 3 of the > > +License, or # (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, # > > +but WITHOUT ANY WARRANTY; without even the implied warranty of # > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # > GNU > > +General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License # > > +along with this program. If not, see > w.gnu.org%2Flicenses%2F&data=04%7C01%7Crupesh.potharla%40amd > .com%7Cc481b41f3aa843656da008d9dd113ffe%7C3dd8961fe4884e608e11a > 82d994e183d%7C0%7C0%7C637783888447831759%7CUnknown%7CTWFpbG > Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > Mn0%3D%7C3000&sdata=qcvs2DAFdVDL3s7Evqm5v6cr8%2FzI1WZ%2B > Htvf1Mq39XA%3D&reserved=0> . > > + > > +# Testing GDB's implementation of ASSUMED RANK. > > + > > +if {[skip_fortran_tests]} { return -1 } > > + > > +standard_testfile ".f90" > > +load_lib fortran.exp > > + > > +if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \ > > + {debug f90}]} { > > In your commit message you made use of the -dwarf-5 flag. Is that required? > I was surprised that this flag doesn't appear here. > There's a couple of tests where we pass 'additional_flags=-gdwarf-5', but you > might only need to do this if the compiler is gfortran, and maybe for > particular versions? > Added 'additional_flags=-gdwarf-5' for gfortran for versions >=11 in the testcase. > I'll try to take a detailed look through next week. > > Thanks, > Andrew