Re: Help! Regarding Bug 17896
On 26 January 2016 at 03:24, Manuel López-Ibáñez wrote: > On 25 January 2016 at 20:17, Mikhail Maltsev wrote: >> As I understand, the bug report suggests that we say "suggest || instead of | >> when joining booleans" instead. We now have the API to show fix-it hints, so >> it >> would be nice to output something like >> >> test.c:17:21: warning: suggest || instead of | when joining booleans >>foo ((a == b) | b == c); >> ^ >> || >> >> Grep 'set_fixit_hint' in the source code to see an example of it's usage. > > I think you should focus on one single very simple thing for a first > patch, either adding fix-it hints to the existing warning or adding > the alternative text. I would in fact suggest to add fix-it hints > without touching anything else, since this is probably less > controversial [*] and more straightforward. How? > > 1) Grep 'set_fixit_hint' in gcc/* gcc/cp/* gcc/c/* gcc/c-family/* and > see how it is used. > 2) Grep 'suggest parentheses around comparison in operand' > 3) Run gcc under gdb and stop at the moment the warning above is given. > 4) Figure out what you need to do to make fix-it hints work in this > case so we give (spaces are probably messed up by gmail): > > test.c:17:17: note: suggest parentheses around comparison in operand of '|' >foo ((a == b) | b == c); > ^ >( ) > > The rest is the standard for submitting patches: > https://gcc.gnu.org/wiki/GettingStarted#Basics:_Contributing_to_GCC_in_10_easy_steps > > This patch is probably a few lines, so you may not even need a > copyright assignment. The goal of starting simple is to learn the > process of submitting a patch without being distracted by the > discussion about the patch itself. Once you feel confident with the > process, you can try changing the text of the warning (but then I > would suggest you find out first what the C/C++ maintainers want > before implementing it). Note that another easyhack > (https://gcc.gnu.org/PR38612) is also a matter of changing the warning > text and the C++ maintainer already gave his blessing to the new text > in comment #5. > > Cheers, > > Manuel. > > [*] I don't agree with the suggestion in the bug report. I think we > should either follow Clang here or we should say something like "& > will evaluate the boolean expression to its right, did you mean '&&'?" > The C/C++ maintainers may even have different opinions. I agree with Manuel (and his comments on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17896) that we should follow clang here and show fix-it hint "& will evaluate the boolean expression to its right, did you mean '&&'?". That will be more clear message than "suggest && instead of & when joining booleans". So I am asking community for their suggestion. -- Thanks and Regards, Prasad Ghangal
Re: RFC: Support non-standard extension (call via casted function pointer)
Richard Biener writes: > I think that's reasonable and what GHC expects - declare "there is a function > foo defined, no idea what prototype yet", unfortunately C requires to specify > a return type. Like void, f.ex. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/28/2016 09:55 AM, Andreas Schwab wrote: > Richard Biener writes: > >> I think that's reasonable and what GHC expects - declare "there is a function >> foo defined, no idea what prototype yet", unfortunately C requires to specify >> a return type. > > Like void, f.ex. Wait. Do you think this would actually allow ghc to determine the return type later? If I remember correctly, ghc currently initially declares the function prototype with return type void*, doesn't it? Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: RFC: Support non-standard extension (call via casted function pointer)
John Paul Adrian Glaubitz writes: > Wait. Do you think this would actually allow ghc to determine the > return type later? If I remember correctly, ghc currently initially > declares the function prototype with return type void*, doesn't it? Replace a lie with a different lie. Spot the pattern? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: RFC: Support non-standard extension (call via casted function pointer)
On 01/27/2016 04:17 PM, Richard Biener wrote:
> We are trying to support
>
> t.c
> ---
> void *foo();
>
> int bar()
> {
> return ((int (*)())foo) ();
> }
>
> t2.c
> -
> int foo () { return 0; }
>
> thus doing a direct call to a function with a (wrong) prototype via a function
> pointer cast to the correct type and having a matching implementation of
> that function elsewhere.
I suspect Ada needs something like this already. I expect the following
to work (although it is quite hideous).
with System;
package Import is
function Foo return System.Address;
pragma Import (C, Foo);
function Bar return Integer;
end Import;
package body Import is
function Bar return Integer is
function Foo_2 return Integer;
pragma Import (C, Foo_2);
for Foo_2'Address use Foo'Address;
begin
return Foo_2;
end Bar;
end Import;
Florian
Re: RFC: Support non-standard extension (call via casted function pointer)
On Thu, Jan 28, 2016 at 11:11 AM, Florian Weimer wrote:
> On 01/27/2016 04:17 PM, Richard Biener wrote:
>
>> We are trying to support
>>
>> t.c
>> ---
>> void *foo();
>>
>> int bar()
>> {
>> return ((int (*)())foo) ();
>> }
>>
>> t2.c
>> -
>> int foo () { return 0; }
>>
>> thus doing a direct call to a function with a (wrong) prototype via a
>> function
>> pointer cast to the correct type and having a matching implementation of
>> that function elsewhere.
>
> I suspect Ada needs something like this already. I expect the following
> to work (although it is quite hideous).
>
> with System;
>
> package Import is
>
>function Foo return System.Address;
>pragma Import (C, Foo);
>
>function Bar return Integer;
>
> end Import;
>
> package body Import is
>
>function Bar return Integer is
> function Foo_2 return Integer;
> pragma Import (C, Foo_2);
> for Foo_2'Address use Foo'Address;
>begin
> return Foo_2;
>end Bar;
>
> end Import;
Heh. Well, the real reason for officially (aka in GCCs middle-end)
supporting this
is LTO and cross-language calls. You hardly can get 1:1 matching declarations
there and penaltizing all of them by forcing us to go through indirect calls
(which backends handle just fine) is no good.
What the middle-end policy does is simply that the generated code should
match the source input call ABI used (which is specified by the function
type a call is carried out on). That makes sense from a QOI perspective anyway.
And yes, it looks like the m68k backend is simply buggy here.
Richard.
>
> Florian
Re: RFC: Support non-standard extension (call via casted function pointer)
> I suspect Ada needs something like this already. I expect the following > to work (although it is quite hideous). > > with System; > > package Import is > >function Foo return System.Address; >pragma Import (C, Foo); > >function Bar return Integer; > > end Import; > > package body Import is > >function Bar return Integer is > function Foo_2 return Integer; > pragma Import (C, Foo_2); > for Foo_2'Address use Foo'Address; >begin > return Foo_2; >end Bar; > > end Import; System.Address is an integer though so that will always work (on 32-bit architectures) without special support. And, no, Ada doesn't need this general capabilitly, address frobbing like above is at your own risk and you certainly don't want to use access (pointer) types with it anyway. That being said, there is indeed a related issue with Ada on m68k because, when you have a C function that returns a pointer (typically malloc), you generally import it in Ada as System.Address: function malloc (Size : size_t) return System.Address; pragma Import (C, malloc, "malloc"); so the C and the Ada sides don't agree on the return register... -- Eric Botcazou
Re: RFC: Support non-standard extension (call via casted function pointer)
> > package Import is > > > >function Foo return System.Address; > >pragma Import (C, Foo); > > > >function Bar return Integer; > > > > end Import; > > > > package body Import is > > > >function Bar return Integer is > > function Foo_2 return Integer; > > pragma Import (C, Foo_2); > > for Foo_2'Address use Foo'Address; BTW the proper idiom would be instead: function Foo_2 return Integer; pragma Import (C, Foo_2, "foo"); to import the same C function with two different types. But as Eric said, users should use this at their own risk, and only on truly compatible types, otherwise this is non portable and may break across compilers or versions. Arno
Re: [RFC PR43721] Optimize a/b and a%b to single divmod call
On Wed, 27 Jan 2016, Prathamesh Kulkarni wrote:
> On 11 November 2015 at 19:04, Richard Biener wrote:
> > On Wed, 11 Nov 2015, Prathamesh Kulkarni wrote:
> >
> >> On 11 November 2015 at 16:03, Richard Biener wrote:
> >> > On Wed, 11 Nov 2015, Prathamesh Kulkarni wrote:
> >> >
> >> >> On 10 November 2015 at 20:11, Richard Biener wrote:
> >> >> > On Mon, 9 Nov 2015, Prathamesh Kulkarni wrote:
> >> >> >
> >> >> >> On 4 November 2015 at 20:35, Richard Biener
> >> >> >> wrote:
> >> >> >> >
> >> >> >> > Btw, did you investigate code gen differences on x86_64/i586? That
> >> >> >> > target expands all divisions/modulo ops via divmod, relying on CSE
> >> >> >> > solely as the HW always computes both div and mod (IIRC).
> >> >> >> x86_64 has optab_handler for divmod defined, so the transform won't
> >> >> >> take place on x86.
> >> >> >
> >> >> > Ok.
> >> >> >
> >> >> >> > +
> >> >> >> > +gassign *assign_stmt = gimple_build_assign
> >> >> >> > (gimple_assign_lhs
> >> >> >> > (use_stmt), rhs);
> >> >> >> > +gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt);
> >> >> >> >
> >> >> >> > Ick. Please use
> >> >> >> >
> >> >> >> > gimple_set_rhs_from_tree (use_stmt, res);
> >> >> >> Um there doesn't seem to be gimple_set_rhs_from_tree.
> >> >> >> I used gimple_assign_set_rhs_from_tree which requires gsi for
> >> >> >> use_stmt.
> >> >> >> Is that OK ?
> >> >> >
> >> >> > Yes.
> >> >> >
> >> >> >> > update_stmt (use_stmt);
> >> >> >> > if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt))
> >> >> >> > cfg_changed = true;
> >> >> >> >
> >> >> >> > + free_dominance_info (CDI_DOMINATORS);
> >> >> >> >
> >> >> >> > do not free dominators.
> >> >> >>
> >> >> >> I have done the suggested changes in the attached patch.
> >> >> >> I have a few questions:
> >> >> >>
> >> >> >> a) Does the change to insert DIVMOD call before topmost div or mod
> >> >> >> stmt with matching operands
> >> >> >> look correct ?
> >> >> >
> >> >> > + /* Insert call-stmt just before the topmost div/mod stmt.
> >> >> > + top_bb dominates all other basic blocks containing div/mod stms
> >> >> > + so, the topmost stmt would be the first div/mod stmt with
> >> >> > matching
> >> >> > operands
> >> >> > + in top_bb. */
> >> >> > +
> >> >> > + gcc_assert (top_bb != 0);
> >> >> > + gimple_stmt_iterator gsi;
> >> >> > + for (gsi = gsi_after_labels (top_bb); !gsi_end_p (gsi); gsi_next
> >> >> > (&gsi))
> >> >> > +{
> >> >> > + gimple *g = gsi_stmt (gsi);
> >> >> > + if (is_gimple_assign (g)
> >> >> > + && (gimple_assign_rhs_code (g) == TRUNC_DIV_EXPR
> >> >> > +|| gimple_assign_rhs_code (g) == TRUNC_MOD_EXPR)
> >> >> > + && operand_equal_p (op1, gimple_assign_rhs1 (g), 0)
> >> >> > + && operand_equal_p (op2, gimple_assign_rhs2 (g), 0))
> >> >> > + break;
> >> >> >
> >> >> > Looks overly complicated to me. Just remember "topmost" use_stmt
> >> >> > alongside top_bb (looks like you'll no longer need top_bb if you
> >> >> > retail top_stmt). And then do
> >> >> >
> >> >> >gsi = gsi_for_stmt (top_stmt);
> >> >> >
> >> >> > and insert before that.
> >> >> Thanks, done in this patch. Does it look OK ?
> >> >> IIUC gimple_uid (stmt1) < gimple_uid (stmt2) can be used to check if
> >> >> stmt1 occurs before stmt2
> >> >> only if stmt1 and stmt2 are in the same basic block ?
> >> >> >
> >> >> >> b) Handling constants - I dropped handling constants in the attached
> >> >> >> patch. IIUC we don't want
> >> >> >> to enable this transform if there's a specialized expansion for some
> >> >> >> constants for div or mod ?
> >> >> >
> >> >> > See expand_divmod which has lots of special cases for constant
> >> >> > operands
> >> >> > not requiring target support for div or mod.
> >> >> Thanks, would it be OK if I do this in follow up patch ?
> >> >
> >> > Well, just not handle them like in your patch is fine.
> >> >
> >> >> >
> >> >> >> I suppose this would also be target dependent and require a target
> >> >> >> hook ?
> >> >> >> For instance arm defines modsi3 pattern to expand mod when 2nd
> >> >> >> operand
> >> >> >> is constant and <= 0 or power of 2,
> >> >> >> while for other cases goes the expand_divmod() route to generate call
> >> >> >> to __aeabi_idivmod libcall.
> >> >> >
> >> >> > Ok, so it lacks a signed mod instruction.
> >> >> >
> >> >> >> c) Gating the divmod transform -
> >> >> >> I tried gating it on checks for optab_handlers on div and mod,
> >> >> >> however
> >> >> >> this doesn't enable transform for arm cortex-a9
> >> >> >> anymore (cortex-a9 doesn't have hardware instructions for integer
> >> >> >> div and mod).
> >> >> >> IIUC for cortex-a9,
> >> >> >> optab_handler (sdivmod_optab, SImode) returns CODE_FOR_nothing
> >> >> >> because
> >> >> >> HAVE_divsi3 is 0.
> >> >> >> However optab_handler (smod_optab, SImode) matches since
> >> >> >> optab_handler
> >> >> >> only checks for existence of pattern
> >> >> >> (and not whet
Compiling with -m64 using attribute interrupt emits IRET not IRETQ
I using hjl/interrupt/gcc-5-branch and my program is crashing when I
issue an INT xx. The problem appears to me to be that using
__attribute__ ((interrupt)) causes the a IRET to be emitted when an
IRETQ should be emitted. Below is my trivial do nothing main.c which I
compile with and then use objdump to view the assembler source.
I compiled x86_64-unknown-elf- using crosstool-ng, could that be a problem?
$ cat main.c
typedef unsigned long u64;
typedef struct intr_frame {
u64 ip;
u64 cs;
u64 flags;
u64 sp;
u64 ss;
} intr_frame;
__attribute__ ((interrupt))
void ih(struct intr_frame* frame) {
}
void main(void) {
}
$ x86_64-unknown-elf-gcc -c main.c -o main.o -m64
$ x86_64-unknown-elf-objdump -d main.o
main.o: file format elf64-x86-64
Disassembly of section .text:
:
0: 55 push %rbp
1: 48 89 e5 mov%rsp,%rbp
4: 50 push %rax
5: 48 8d 45 08 lea0x8(%rbp),%rax
9: 48 89 45 f0 mov%rax,-0x10(%rbp)
d: 90 nop
e: 58 pop%rax
f: 5d pop%rbp
10: cf iret
0011 :
11: 55 push %rbp
12: 48 89 e5 mov%rsp,%rbp
15: 90 nop
16: 5d pop%rbp
17: c3 retq
Re: RFC: Support non-standard extension (call via casted function pointer)
On 28.01.2016 11:04, Andreas Schwab wrote: > John Paul Adrian Glaubitz writes: > >> [suggestion to use "void" as dummy return type] >> >> Wait. Do you think this would actually allow ghc to determine the >> return type later? If I remember correctly, ghc currently initially >> declares the function prototype with return type void*, doesn't it? > Replace a lie with a different lie. Spot the pattern? > > Andreas. I am sorry, I fail to spot any pattern. As I understood you, you are opposed to changing gcc because a program that lies to gcc fails to get the result this program expects. But I don't see any pattern in "replacing a lie with a different lie". Please be more specific in what your message should tell the recipients. In case you refer to the ppc64el issue of ghc, and try to imply that changing lies at it fits has precedence in ghc history (and thus ghc either needs to stop lying or find a new lie that just happens to work), I strongly disagree. In that case, ghc developers not "replace a lie with a different lie", but replaced a lie "(empty parameter list)" with the truth "(unknown parameter list)". Regards, Michael Karcher
Re: RFC: Support non-standard extension (call via casted function pointer)
Why does the dummy declaration need to use a pointer type? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
libstdc++ and c library compatible issue when bootstrap GCC
Hi, I ran into below error message at stage2 of bootstrap GCC: /work/obj/gcc-bootstrap/./prev-gcc/xg++ -B/work/obj/gcc-bootstrap/./prev-gcc/ -B//aarch64-none-linux-gnu/bin/ -nostdinc++ -B/work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/src/.libs -B/work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/libsupc++/.libs -I/work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/include/aarch64-none-linux-gnu -I/work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/include -I/src/gcc/libstdc++-v3/libsupc++ -L/work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/src/.libs -L/work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/libsupc++/.libs -c -g -O2 -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -fno-PIE -I. -Ibuild -I/src/gcc/gcc -I/src/gcc/gcc/build -I/src/gcc/gcc/../include -I/src/gcc/gcc/../libcpp/include \ -o build/genautomata.o /src/gcc/gcc/genautomata.c In file included from /src/gcc/gcc/genautomata.c:116:0: /work/obj/gcc-bootstrap/prev-aarch64-none-linux-gnu/libstdc++-v3/include/math.h:65:12: error: ‘constexpr bool std::isinf(double)’ conflicts with a previous declaration using std::isinf; ^ In file included from /usr/include/features.h:374:0, from /usr/include/stdio.h:27, from /src/gcc/gcc/system.h:46, from /src/gcc/gcc/genautomata.c:108: /usr/include/aarch64-linux-gnu/bits/mathcalls.h:201:1: note: previous declaration ‘int isinf(double)’ __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__)); ^ I kind of understand what the problem is base on below facts: 1) I bootstrap gcc against new glibc; 2) I configure gcc with "--with-build-sysroot". According to GCC document, this only affects how target libraries are built. 3) binary executables are built in GCC for internal use purpose. These binaries are used to generate internal files during building gcc, and I guess they are not considered as target library. So system C libraries/headers are used. 4) at stage2 of gcc bootstrap, these binaries are compiled by stage1 xg++, which by default uses new libstdc+ just built in stage1. 5) Problem, the new libstdc++ is configured/built against new glibc because of "--with-build-sysroot". It's incompatible with system C headers. It's a little tricky. First question is: is it reasonable to use "--with-build-sysroot" alone when building/bootstrapping gcc? If yes, how should we build binaries in GCC? Seems to me it's not reasonable to use new libstdc++ along with system C library since it's built against new glibc. Any suggestion? Thanks in advance. Thanks, bin
Re: Compiling with -m64 using attribute interrupt emits IRET not IRETQ
On Thu, Jan 28, 2016 at 9:06 AM, Wink Saville wrote: > I using hjl/interrupt/gcc-5-branch and my program is crashing when I > issue an INT xx. The problem appears to me to be that using > __attribute__ ((interrupt)) causes the a IRET to be emitted when an > IRETQ should be emitted. Below is my trivial do nothing main.c which I > compile with and then use objdump to view the assembler source. > > I compiled x86_64-unknown-elf- using crosstool-ng, could that be a > problem? I fixed it on hjl/interrupt/gcc-5-branch branch. I will fix it for hjl/interrupt/stage1 soon. Thanks. -- H.J.
Re: Compiling with -m64 using attribute interrupt emits IRET not IRETQ
On Thu, Jan 28, 2016 at 10:26 AM, H.J. Lu wrote: > On Thu, Jan 28, 2016 at 9:06 AM, Wink Saville wrote: >> I using hjl/interrupt/gcc-5-branch and my program is crashing when I >> issue an INT xx. The problem appears to me to be that using >> __attribute__ ((interrupt)) causes the a IRET to be emitted when an >> IRETQ should be emitted. Below is my trivial do nothing main.c which I >> compile with and then use objdump to view the assembler source. >> >> I compiled x86_64-unknown-elf- using crosstool-ng, could that be a >> problem? > > I fixed it on hjl/interrupt/gcc-5-branch branch. I will fix it for > hjl/interrupt/stage1 soon. > > hjl/interrupt/stage1 branch is also fixed. -- H.J.
Re: Compiling with -m64 using attribute interrupt emits IRET not IRETQ
On Thu, Jan 28, 2016 at 11:06 AM, Wink Saville wrote:
> Thanks. How are you testing?
>
>
* { dg-do compile } */
/* { dg-options "-O2 -Wall -g" } */
void
__attribute__((interrupt))
fn (void *frame)
{
}
/* { dg-final { scan-assembler-not "add(l|q)\[\\t \]*\\$\[0-9\]*,\[\\t
\]*%\[re\]?sp" } } */
/* { dg-final { scan-assembler "iret" { target ia32 } } } */
/* { dg-final { scan-assembler "iretq" { target { ! ia32 } } } } */
^^^
The testcase scans for iretq if target isn't 32-bit.
--
H.J.
Strange diagnostics behavior with patch
Hello world,
the attached patch fixes the regression of PR 60526 by checking for
the presence of a type with the same name as the variable. Types
effectively have their separate namespace because the names of their
symtrees start with an uppercase letter. So far, so good.
However, the error message generated is far to big, and contains
statements which are not relevant and also empty lines.
For the test case
type xx
end type
integer :: q
real :: a
integer, parameter :: h=3
type(xX) :: Xx
end
the error message becomes
type3.f90:8:14:
type xx
2
end type
integer :: q
real :: a
integer, parameter :: h=3
type(xX) :: Xx
1
Error: Symbol »xx« at (1) also declared as a type at (2)
The error message is emitted via
gfc_error ("Symbol %qs at %C also declared as a type at %L", name,
&st->n.sym->declared_at);
which I think is the right thing to do. Am I using this wrong, or is it
a quirk in gfc_error or in the general error handling routines?
Regards
Thomas
Index: decl.c
===
--- decl.c (Revision 232864)
+++ decl.c (Arbeitskopie)
@@ -1215,10 +1215,32 @@ build_sym (const char *name, gfc_charlen *cl, bool
{
symbol_attribute attr;
gfc_symbol *sym;
+ char *u_name;
+ int nlen;
+ gfc_symtree *st;
if (gfc_get_symbol (name, NULL, &sym))
return false;
+ /* Check if the name has already been defined as a type. The
+ first letter of the symtree will be in upper case then. */
+
+ nlen = strlen(name);
+
+ u_name = XCNEWVEC(char, nlen+1);
+ u_name[0] = TOUPPER(name[0]);
+ strncpy (u_name+1, name+1, nlen);
+
+ st = gfc_find_symtree (gfc_current_ns->sym_root, u_name);
+ free (u_name);
+
+ if (st != 0)
+{
+ gfc_error ("Symbol %qs at %C also declared as a type at %L", name,
+ &st->n.sym->declared_at);
+ return false;
+}
+
/* Start updating the symbol table. Add basic type attribute if present. */
if (current_ts.type != BT_UNKNOWN
&& (sym->attr.implicit_type == 0
Re: Strange diagnostics behavior with patch
On Thu, 2016-01-28 at 21:07 +0100, Thomas Koenig wrote:
> Hello world,
>
> the attached patch fixes the regression of PR 60526 by checking for
> the presence of a type with the same name as the variable. Types
> effectively have their separate namespace because the names of their
> symtrees start with an uppercase letter. So far, so good.
>
> However, the error message generated is far to big, and contains
> statements which are not relevant and also empty lines.
>
> For the test case
>
> type xx
> end type
>
> integer :: q
> real :: a
> integer, parameter :: h=3
>
> type(xX) :: Xx
>
> end
>
>
> the error message becomes
>
> type3.f90:8:14:
>
> type xx
> 2
> end type
>
>
>
> integer :: q
>
> real :: a
>
> integer, parameter :: h=3
>
>
>
> type(xX) :: Xx
>1
> Error: Symbol »xx« at (1) also declared as a type at (2)
>
> The error message is emitted via
>
>gfc_error ("Symbol %qs at %C also declared as a type at %L",
> name,
> &st->n.sym->declared_at);
>
> which I think is the right thing to do. Am I using this wrong, or is
> it
> a quirk in gfc_error or in the general error handling routines?
This looks like a bug in the general error-handling routines: for
diagnostics containing more than one location, the new implementation
of diagnostic_show_locus attempts to show all of the relevant locations
in one loop. It prints a range of lines covering all of the locations
within the same source file as the primary location of the diagnostic.
It looks like we need some logic to split things up if there's a big
gap between locations within one diagnostic (and probably to only print
annotation lines under the source if there are some annotations to go
in those lines).
Please can you file this example in bugzilla and CC me?
Thanks
Dave
Re: [RFC PR43721] Optimize a/b and a%b to single divmod call
On Thu, Jan 28, 2016 at 5:37 AM, Richard Biener wrote: >> To workaround this, I defined a new hook expand_divmod_libfunc, which >> targets must override for expanding call to target-specific dimovd. >> The "default" hook default_expand_divmod_libfunc() expands call to >> libgcc2.c:__udivmoddi4() since that's the only "generic" divmod >> available. >> Is this a reasonable approach ? > > Hum. How do they get to expand/generate it today? That said, I'm > no expert in this area. Currently, the only place where a divmod libfunc can be called is in expand_divmod in expmed.c, which can return either the div or mod result, but not both. If this is called for the mod result, and there is no div insn, and no mod insn, and no mod libfunc, then it will call the divmod libfunc to generate the mod result. This is exactly the case where the ARM port needs it, as this code was written for the arm. There are 3 targets that define a divmod libfunc: arm, c6x, and spu. The arm port is OK, because expand_divmod does the right thing for arm, using the arm divmod calling convention. The c6x port is OK because it defines mod insns and libfuncs, and hence the divmod libfunc will never be called and is redundant. The spu port is also OK, because it defines mod libcalls, and hence the divmod libfunc will never be called, and is likewise redundant. Both the c6x and spu ports have their own divmod library functions in libgcc/config/$target. The divmod library functions are called by the div and mod library functions, so they are necessary, they are just never directly called. Both the c6x and spu port uses the current libgcc __udivmoddi4 calling convention with a pointer to the mod result, which is different and incompatible to the ARM convention of returning a double size result that contains div and mod. WIth Prathamesh's patch to add support to the tree optimizers to create divmod operations, the c6x and spu ports break. The divmod libfuncs are no longer redundant, and will be called, except with the wrong ABI, so we need to extend the divmod support to handle multiple ABIs. This is why Prathamesh added the target hook for the divmod libcall, so the target can specify the ABI used by its divmod libcalls. Prathamesh has correct support for ARM (current code), and apparently correct code for c6x and spu (libgcc udivmodsi4). > A simpler solution may be to not do the transform if there is no > instruction for divmod. This prevents the optimization from happening on ARM, which has divmod libfuncs but no divmod insn. We want the optimization to happen there, as if we need both div and mod results, then calling the divmod libfunc is faster than calling both the div and mod libfuncs. Jim
