[committed] openmp: Diagnose threadprivate OpenMP loop iterators

2021-10-30 Thread Jakub Jelinek via Gcc-patches 
Hi!

We weren't diagnosing the
The loop iteration variable may not appear in a threadprivate directive.
restriction which used to be in 5.0 just among the Worksharing-Loop
restrictions but in 5.1 it is among Canonical Loop Nest Form restrictions.

This patch diagnoses those.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2021-10-30  Jakub Jelinek  

* gimplify.c (gimplify_omp_for): Diagnose threadprivate iterators.

* c-c++-common/gomp/loop-10.c: New test.

--- gcc/gimplify.c.jj   2021-10-07 12:52:35.195909331 +0200
+++ gcc/gimplify.c  2021-10-29 19:42:34.343358109 +0200
@@ -12298,6 +12298,24 @@ gimplify_omp_for (tree *expr_p, gimple_s
  gimplify_omp_ctxp->loop_iter_var.quick_push (decl);
}
 
+  if (for_stmt == orig_for_stmt)
+   {
+ tree orig_decl = decl;
+ if (OMP_FOR_ORIG_DECLS (for_stmt))
+   {
+ tree orig_decl = TREE_VEC_ELT (OMP_FOR_ORIG_DECLS (for_stmt), i);
+ if (TREE_CODE (orig_decl) == TREE_LIST)
+   {
+ orig_decl = TREE_PURPOSE (orig_decl);
+ if (!orig_decl)
+   orig_decl = decl;
+   }
+   }
+ if (is_global_var (orig_decl) && DECL_THREAD_LOCAL_P (orig_decl))
+   error_at (EXPR_LOCATION (for_stmt),
+ "threadprivate iteration variable %qD", orig_decl);
+   }
+
   /* Make sure the iteration variable is private.  */
   tree c = NULL_TREE;
   tree c2 = NULL_TREE;
--- gcc/testsuite/c-c++-common/gomp/loop-10.c.jj2021-10-29 
20:38:28.971214355 +0200
+++ gcc/testsuite/c-c++-common/gomp/loop-10.c   2021-10-29 20:42:40.637710320 
+0200
@@ -0,0 +1,35 @@
+int a, b;
+#pragma omp threadprivate (a, b)
+
+void
+foo (void)
+{
+  #pragma omp for  /* { dg-error "threadprivate 
iteration variable 'a'" } */
+  for (a = 0; a < 32; a++)
+;
+  #pragma omp parallel for collapse(2) /* { dg-error "threadprivate 
iteration variable 'a'" "" { target c } } */
+  for (a = 0; a < 32; a++) /* { dg-error "threadprivate 
iteration variable 'b'" "" { target c } .-1 } */
+for (b = 0; b < 32; b++)   /* { dg-error "threadprivate 
iteration variable 'a'" "" { target c++ } .-1 } */
+  ;/* { dg-error 
"threadprivate iteration variable 'b'" "" { target c++ } .-2 } */
+  #pragma omp simd /* { dg-error "threadprivate 
iteration variable 'a'" } */
+  for (a = 0; a < 32; a++)
+;
+  #pragma omp taskloop /* { dg-error "threadprivate 
iteration variable 'a'" } */
+  for (a = 0; a < 32; a++)
+;
+  #pragma omp loop bind(thread)/* { dg-error 
"threadprivate iteration variable 'a'" } */
+  for (a = 0; a < 32; a++)
+;
+}
+
+void
+bar (void)
+{
+  #pragma omp distribute collapse(2)   /* { dg-error "threadprivate 
iteration variable 'a'" } */
+  for (a = 0; a < 32; a++) /* { dg-error "threadprivate 
iteration variable 'b'" "" { target *-*-* } .-1 } */
+for (b = 0; b < a; b++)
+  ;
+  #pragma omp distribute parallel for simd /* { dg-error "threadprivate 
iteration variable 'a'" "" { target c } } */
+  for (a = 0; a < 32; a++) /* { dg-error "threadprivate 
iteration variable 'a'" "" { target c++ } } */
+;
+}

Jakub

Re: [PATCH] hardened conditionals

2021-10-30 Thread Alexandre Oliva via Gcc-patches 
FYI, I'm putting in this follow-up tweak to the GNAT manual.


Implied compares in Ada Harded Conditionals documentation

From: Alexandre Oliva 

Improve the wording on optimizations that prevent compare hardening,
so as to also cover cases in which explicit compares get combined into
operations with implied compares.


for  gcc/ada/ChangeLog

* doc/gnat_rm/security_hardening_features.rst: Mention
optimization to operations with implied compares.
---
 .../doc/gnat_rm/security_hardening_features.rst|7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
index 52240d7e3dd54..cf76938d91d13 100644
--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
@@ -112,14 +112,15 @@ long after boolean expressions are decomposed into 
separate compares,
 each one turned into either a conditional branch or a compare whose
 result is stored in a boolean variable or temporary.  Compiler
 optimizations, if enabled, may also turn conditional branches into
-stored compares, and vice-versa.  Conditionals may also be optimized
+stored compares, and vice-versa, or into operations with implied
+conditionals (e.g. MIN and MAX).  Conditionals may also be optimized
 out entirely, if their value can be determined at compile time, and
 occasionally multiple compares can be combined into one.
 
 It is thus difficult to predict which of these two options will affect
 a specific compare operation expressed in source code.  Using both
-options ensures that every compare that is not optimized out will be
-hardened.
+options ensures that every compare that is neither optimized out nor
+optimized into implied conditionals will be hardened.
 
 The addition of reversed compares can be observed by enabling the dump
 files of the corresponding passes, through command line options


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about

[PATCH] x86_64: Expand ashrv1ti (and PR target/102986)

2021-10-30 Thread Roger Sayle 

This patch was originally intended to implement 128-bit arithmetic right
shifts by constants of vector registers (V1TImode), but while working on
it I discovered the (my) recent ICE on valid regression now known as
PR target/102986.

As diagnosed by Jakub, expanders for shifts are not allowed to fail, and
so any backend that provides a shift optab needs to handle variable amount
shifts as well as constant shifts [even though the middle-end knows how
to synthesize these for vector modes].  This constraint could be relaxed
in the middle-end, but it makes sense to fix this in my erroneous code.

The solution is to change the constraints on the recently added (and new)
shift expanders from SImode const_int_register to QImode general operand,
matching the TImode expanders' constraints, and then simply check for
!CONST_INT_P at the start of the ix86_expand_v1ti_* functions, converting
the operands from V1TImode to TImode, performing the TImode operation
and converting the result back to V1TImode.

One nice benefit of this strategy, is that it allows us to implement
Uros' recent suggestion, that we should be more efficiently converting
between these modes, avoiding the use of memory and using the same idiom
as LLVM or using pextrq/pinsrq where available.  The new helper functions
ix86_expand_v1ti_to_ti and ix86_expand_ti_to_v1ti are sufficient to take
care of this.  Interestingly partial support for this is already present,
but x86_64's generic tuning prefers memory transfers to avoid penalizing
microarchitectures with significant interunit delays.  With these changes
we not generate both pextrq and pinsrq for -mtune=native.

The main body of the patch is to implement arithmetic right shift in
addition to the logical right shift and left shift implemented in the
previous patch.  This expander provides no less than 13 different code
sequences, special casing the different constant shifts, including
variants taking advantage of TARGET_AVX2 and TARGET_SSE4_1.  The code
is structured with the faster/shorter sequences and the start, and
the generic implementations at the end.

For the record, the implementations are:

ashr_127:   // Shift 127, 2 operations, 10 bytes
pshufd  $255, %xmm0, %xmm0
psrad   $31, %xmm0
ret

ashr_64:// Shift by 64, 3 operations, 14 bytes
pshufd  $255, %xmm0, %xmm1
psrad   $31, %xmm1
punpckhqdq  %xmm1, %xmm0
ret

ashr_96:// Shift by 96, 3 operations, 18 bytes
movdqa  %xmm0, %xmm1
psrad   $31, %xmm1
punpckhqdq  %xmm1, %xmm0
pshufd  $253, %xmm0, %xmm0
ret

ashr_8: // Shift by 8/16/24/32 on AVX2, 3 operations, 16 bytes
vpsrad  $8, %xmm0, %xmm1
vpsrldq $1, %xmm0, %xmm0
vpblendd$7, %xmm0, %xmm1, %xmm0
ret

ashr_8: // Shift by 8/16/24/32 on SSE4.1, 3 operations, 24 bytes
movdqa  %xmm0, %xmm1
psrldq  $1, %xmm0
psrad   $8, %xmm1
pblendw $63, %xmm0, %xmm1
movdqa  %xmm1, %xmm0
ret

ashr_97:// Shifts by 97..126, 4 operations, 23 bytes
movdqa  %xmm0, %xmm1
psrad   $31, %xmm0
psrad   $1, %xmm1
punpckhqdq  %xmm0, %xmm1
pshufd  $253, %xmm1, %xmm0
ret

ashr_48:// Shifts by 48/80 on SSE4.1, 4 operations, 25 bytes
movdqa  %xmm0, %xmm1
pshufd  $255, %xmm0, %xmm0
psrldq  $6, %xmm1
psrad   $31, %xmm0
pblendw $31, %xmm1, %xmm0
ret

ashr_8: // Shifts by multiples of 8, 5 operations, 28 bytes
movdqa  %xmm0, %xmm1
pshufd  $255, %xmm0, %xmm0
psrad   $31, %xmm0
psrldq  $1, %xmm1
pslldq  $15, %xmm0
por %xmm1, %xmm0
ret

ashr_1: // Shifts by 1..31 on AVX2, 6 operations, 30 bytes
vpsrldq $8, %xmm0, %xmm2
vpsrad  $1, %xmm0, %xmm1
vpsllq  $63, %xmm2, %xmm2
vpsrlq  $1, %xmm0, %xmm0
vpor%xmm2, %xmm0, %xmm0
vpblendd$7, %xmm0, %xmm1, %xmm0
ret

ashr_1: // Shifts by 1..15 on SSE4.1, 6 operations, 42 bytes
movdqa  %xmm0, %xmm2
movdqa  %xmm0, %xmm1
psrldq  $8, %xmm2
psrlq   $1, %xmm0
psllq   $63, %xmm2
psrad   $1, %xmm1
por %xmm2, %xmm0
pblendw $63, %xmm0, %xmm1
movdqa  %xmm1, %xmm0
ret

ashr_1: // Shift by 1, 8 operations, 46 bytes
movdqa  %xmm0, %xmm1
movdqa  %xmm0, %xmm2
psrldq  $8, %xmm2
psrlq   $63, %xmm1
psllq   $63, %xmm2
psrlq   $1, %xmm0
pshufd  $191, %xmm1, %xmm1
por %xmm2, %xmm0
psllq   $31, %xmm1
por %xmm1, %xmm0
ret

ashr_65:// Shifts by 65..95, 8 operations, 42 bytes
pshufd  $255, %xmm0, %xmm1
psrldq  $8, %xmm0
psrad   $31, %xmm1
psrlq   $1, %xmm0
movdqa  %xmm1, %xmm2
psllq   $63, %xmm1

Re: [PATCH] x86_64: Expand ashrv1ti (and PR target/102986)

2021-10-30 Thread Jakub Jelinek via Gcc-patches 
On Sat, Oct 30, 2021 at 11:16:41AM +0100, Roger Sayle wrote:
> 2021-10-30  Roger Sayle  
> 
> gcc/ChangeLog
>   PR target/102986
>   * config/i386/i386-expand.c (ix86_expand_v1ti_to_ti,
>   ix86_expand_ti_to_v1ti): New helper functions.
>   (ix86_expand_v1ti_shift): Check if the amount operand is an
>   integer constant, and expand as a TImode shift if it isn't.
>   (ix86_expand_v1ti_rotate): Check if the amount operand is an
>   integer constant, and expand as a TImode rotate if it isn't.
>   (ix86_expand_v1ti_ashiftrt): New function to expand arithmetic
>   right shifts of V1TImode quantities.
>   * config/i386/i386-protos.h (ix86_expand_v1ti_ashift): Prototype.
>   * config/i386/sse.md (ashlv1ti3, lshrv1ti3): Change constraints
>   to QImode general_operand, and let the helper functions lower
>   shifts by non-constant operands, as TImode shifts.
>   (ashrv1ti3): New expander calling ix86_expand_v1ti_ashiftrt.
>   (rotlv1ti3, rotrv1ti3): Change shift operand to QImode.
> 
> gcc/testsuite/ChangeLog
>   PR target/102986
>   * gcc.target/i386/sse2-v1ti-ashiftrt-1.c: New test case.
>   * gcc.target/i386/sse2-v1ti-ashiftrt-2.c: New test case.
>   * gcc.target/i386/sse2-v1ti-ashiftrt-3.c: New test case.
>   * gcc.target/i386/sse2-v1ti-shift-2.c: New test case.
>   * gcc.target/i386/sse2-v1ti-shift-3.c: New test case.
> 
> Sorry again for the breakage in my last patch.   I wasn't testing things
> that shouldn't have been affected/changed.

Not a review, will defer that to Uros, but just nits:

> +/* Expand move of V1TI mode register X to a new TI mode register.  */
> +static rtx ix86_expand_v1ti_to_ti (rtx x)

ix86_expand_v1ti_to_ti should be at the start of next line, so
static rtx
ix86_expand_v1ti_to_ti (rtx x)

Ditto for other functions and also in functions you've added by the
previous patch.
> +  emit_insn (code == ASHIFT ? gen_ashlti3(tmp2, tmp1, operands[2])
> + : gen_lshrti3(tmp2, tmp1, operands[2]));

Space before ( twice.

> +  emit_insn (code == ROTATE ? gen_rotlti3(tmp2, tmp1, operands[2])
> + : gen_rotrti3(tmp2, tmp1, operands[2]));

Likewise.

> +  emit_insn (gen_ashrti3(tmp2, tmp1, operands[2]));

Similarly.

Also, I wonder for all these patterns (previously and now added), shouldn't
they have && TARGET_64BIT in conditions?  I mean, we don't really support
scalar TImode for ia32, but VALID_SSE_REG_MODE includes V1TImode and while
the constant shifts can be done, I think the variable shifts can't, there
are no TImode shift patterns...

Jakub

[PATCH] IBM Z: Fix address of operands will never be NULL warnings

2021-10-30 Thread Stefan Schulze Frielinghaus via Gcc-patches 
Since a recent enhancement of -Waddress a couple of warnings are emitted
and turned into errors during bootstrap:

gcc/config/s390/s390.md:12087:25: error: the address of 'operands' will never 
be NULL [-Werror=address]
12087 |   "TARGET_HTM && operands != NULL
build/gencondmd.c:59:12: note: 'operands' declared here
   59 | extern rtx operands[];
  |^~~~

Fixed by removing those non-null checks.
Bootstrapped and regtested on IBM Z.  Ok for mainline?

gcc/ChangeLog:

* config/s390/s390.md ("*cc_to_int", "tabort", "*tabort_1",
"*tabort_1_plus"): Remove operands non-null check.
---
 gcc/config/s390/s390.md | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index b8bdbaec468..4debdcd1247 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -3533,7 +3533,7 @@
   [(set (match_operand:SI 0 "nonimmediate_operand" "=d")
 (unspec:SI [(match_operand 1 "register_operand" "0")]
UNSPEC_CC_TO_INT))]
-  "operands != NULL"
+  ""
   "#"
   "reload_completed"
   [(set (match_dup 0) (lshiftrt:SI (match_dup 0) (const_int 28)))])
@@ -12062,7 +12062,7 @@
 (define_expand "tabort"
   [(unspec_volatile [(match_operand:SI 0 "nonmemory_operand" "")]
UNSPECV_TABORT)]
-  "TARGET_HTM && operands != NULL"
+  "TARGET_HTM"
 {
   if (CONST_INT_P (operands[0])
   && INTVAL (operands[0]) >= 0 && INTVAL (operands[0]) <= 255)
@@ -12076,7 +12076,7 @@
 (define_insn "*tabort_1"
   [(unspec_volatile [(match_operand:SI 0 "nonmemory_operand" "aJ")]
UNSPECV_TABORT)]
-  "TARGET_HTM && operands != NULL"
+  "TARGET_HTM"
   "tabort\t%Y0"
   [(set_attr "op_type" "S")])
 
@@ -12084,8 +12084,7 @@
   [(unspec_volatile [(plus:SI (match_operand:SI 0 "register_operand"  "a")
  (match_operand:SI 1 "const_int_operand" "J"))]
UNSPECV_TABORT)]
-  "TARGET_HTM && operands != NULL
-   && CONST_OK_FOR_CONSTRAINT_P (INTVAL (operands[1]), 'J', \"J\")"
+  "TARGET_HTM && CONST_OK_FOR_CONSTRAINT_P (INTVAL (operands[1]), 'J', \"J\")"
   "tabort\t%1(%0)"
   [(set_attr "op_type" "S")])
 
-- 
2.31.1

Re: [PATCH] PR fortran/99853 - ICE: Cannot convert 'LOGICAL(4)' to 'INTEGER(8)' (etc.)

2021-10-30 Thread Harald Anlauf via Gcc-patches 

Committed as simple and obvious after discussion in PR.

Harald

Am 28.10.21 um 23:03 schrieb Harald Anlauf via Fortran:

Dear Fortranners,

the original fix by Steve was lingering in the PR.

We did ICE in situations where in a SELECT CASE a kind conversion
was deemed necessary, but it did involve different types.
The check gfc_convert_type_warn () was invoked with arguments
requesting to generate an internal error.  A regular gfc_error
is good enough here.

Regtested on x86_64-pc-linux-gnu.  OK?

Thanks, also to Steve,

Harald


Fortran: generate regular error on invalid conversions of CASE expressions

gcc/fortran/ChangeLog:

PR fortran/99853
* resolve.c (resolve_select): Generate regular gfc_error on
invalid conversions instead of an gfc_internal_error.

gcc/testsuite/ChangeLog:

PR fortran/99853
* gfortran.dg/pr99853.f90: New test.

Re: [PATCH] Fortran: recognize Gerhard Steinmetz

2021-10-30 Thread Harald Anlauf via Gcc-patches 

Committed as simple and obvious as r12-4803.

Harald

Am 30.10.21 um 01:18 schrieb Manfred Schwarb via Fortran:

Am 29.10.21 um 21:58 schrieb Harald Anlauf via Fortran:

Hi Manfred,

Am 29.10.21 um 16:33 schrieb Manfred Schwarb via Fortran:

Hi,
there were really a lot of test cases provided by Gerhard Steinmetz lately.
Although I'm not really in the position to suggest this,
I would appreciate it, if one could recognize him by adding an entry to 
gfortran.texi.

As e.g. in the proposed patch. Such a patch should probably be signed-off my 
someone of
the inner circle and not by me ;-)


well, this is sth. close to obvious to everybody. ;-)
Anyway, a ChangeLog entry would be nice.


I would feel more comfortable if such a patch would origin from somebody else, 
not from me, actually.



Harald


Cheers,
Manfred

Re: [PATCH] Fortran: adjust column sizes in intrinsic.texi

2021-10-30 Thread Harald Anlauf via Gcc-patches 

Committed as r12-4805.

Thanks for the patch!

Harald

Am 30.10.21 um 01:14 schrieb Manfred Schwarb via Fortran:

Am 29.10.21 um 21:44 schrieb Harald Anlauf via Fortran:

Hi Manfred,

Am 29.10.21 um 16:05 schrieb Manfred Schwarb via Fortran:

Hi,

in intrinsic.texi, a lot of tables wrap lines when watching the
resulting info file in a 80char terminal.

Adjust the @columnfractions items to fit screen. Some minor white space
changes are added as well to help saving space.


the patch looks fine.  However, could you please provide a properly
formatted patch that applies using git patch and a ChangeLog entry?



Sorry, forgot the changelog entry, I added it to the patch now.


Thanks,
Harald


Signed-off-by Manfred Schwarb 


[Note: I do not have commit access]

Re: [PATCH] Fortran: Correct documentation for REAL intrinsic

2021-10-30 Thread Harald Anlauf via Gcc-patches 

Committed as r12-4806.

Thanks for the patch!

Harald

Am 30.10.21 um 01:17 schrieb Manfred Schwarb via Fortran:

Am 29.10.21 um 21:56 schrieb Harald Anlauf via Fortran:

Hi Manfred,

Am 29.10.21 um 16:18 schrieb Manfred Schwarb via Gcc-patches:

Hi,

documentation for REAL intrinsic is slightly wrong. Fix it.
Patch is done on top of the column adjustment patch.


the patch looks fine, but it would help a lot to have a ChangeLog entry.


Sorry, forgot the changelog entry, I added it to the patch now.



Thanks,
Harald


Signed-off-by Manfred Schwarb 


[Note: I do not have commit access]

Re: [PATCH] Fortran: adjust error message for SHORT and LONG intrinsics

2021-10-30 Thread Harald Anlauf via Gcc-patches 

Committed as r12-4807.

Thanks for the patch!

Harald

Am 30.10.21 um 01:15 schrieb Manfred Schwarb via Gcc-patches:

Am 29.10.21 um 21:51 schrieb Harald Anlauf via Fortran:

Hi Manfred,

Am 29.10.21 um 16:12 schrieb Manfred Schwarb via Fortran:

Hi,

on 2019-07-23, support for SHORT and LONG intrinsics were removed be Steve 
Kargl by
adding an error message in check.c.  However, the error message
Error: 'long' intrinsic subprogram at (1) has been deprecated
is misleading, as support has been disabled by this patch.

Adjust the error message. This error message does not appear in the testsuite 
AFAIK.


the patch looks fine.  A testcase checking the error message is missing,
as well as a ChangeLog entry.


Sorry, forgot the changelog entry, I added it to the patch now.
Testcase was missing already before, but I added a trivial test to the patch 
for completeness.



Thanks,
Harald


Signed-off-by Manfred Schwarb 


[Note: I do not have commit access]

Re: [PATCH] Fortran: Remove documentation for SHORT and LONG intrinics

2021-10-30 Thread Harald Anlauf via Gcc-patches 

Committed as r12-4808 after checking "make dvi".

Thanks for the patch!

Harald

Am 30.10.21 um 01:16 schrieb Manfred Schwarb via Gcc-patches:

Am 29.10.21 um 21:52 schrieb Harald Anlauf via Fortran:

Hi Manfred,

Am 29.10.21 um 16:13 schrieb Manfred Schwarb via Gcc-patches:

Hi,

on 2019-07-23, support for SHORT and LONG intrinsics was removed be Steve Kargl 
by
adding an error message in check.c.  As far as I can see code support is still 
there, though.

Remove documentation for these intrinsics.


could you please provide a formatted patch that applies using git apply?
And a ChangeLog entry?


Sorry, forgot the changelog entry, I added it to the patch now.



Thanks,
Harald


Signed-off-by Manfred Schwarb 


[Note: I do not have commit access]

Re: [PATCH,FORTRAN 01/29] gdbinit: break on gfc_internal_error

2021-10-30 Thread Bernhard Reutner-Fischer via Gcc-patches 
On 30 October 2021 00:13:06 CEST, Jerry D  wrote:
>Looks OK.

Thanks!
I guess I need an OK from some global maintainer, too?

The breakpoint is ignored by automatically answering the question with n if the 
symbol is not found when loading .gdbinit for e.g. cc1.

thanks,
>
>Cheers
>
>On 10/29/21 11:58 AM, Bernhard Reutner-Fischer via Fortran wrote:
>> ping
>>
>> On Wed,  5 Sep 2018 14:57:04 +
>> Bernhard Reutner-Fischer  wrote:
>>
>>> From: Bernhard Reutner-Fischer 
>>>
>>> Aids debugging the fortran FE.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2017-11-12  Bernhard Reutner-Fischer  
>>>
>>> * gdbinit.in: Break on gfc_internal_error.
>>> ---
>>>   gcc/gdbinit.in | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in
>>> index 4db977f0bab..ac4d7c42e21 100644
>>> --- a/gcc/gdbinit.in
>>> +++ b/gcc/gdbinit.in
>>> @@ -227,6 +227,7 @@ b fancy_abort
>>>   
>>>   # Put a breakpoint on internal_error to help with debugging ICEs.
>>>   b internal_error
>>> +b gfc_internal_error
>>>   
>>>   set complaints 0
>>>   # Don't let abort actually run, as it will make
>

Re: [PATCH] Add a simulate_record_decl lang hook

2021-10-30 Thread Richard Sandiford via Gcc-patches 
Jason Merrill  writes:
> On 10/18/21 16:35, Richard Sandiford wrote:
>> Jason Merrill  writes:
>>> On 9/24/21 13:53, Richard Sandiford wrote:
 +  if (type == error_mark_node)
 +return lhd_simulate_record_decl (loc, name, fields);
>>>
>>> Why fall back to the language-independent function on error?  Is there a
>>> case where that gives better error recovery than just returning
>>> error_mark_node?
>> 
>> I don't think falling back necessarily improves future error messages
>> (or makes them worse).  The reason was more that the code to handle
>> target builtins generally expects to be able to create whatever types
>> and functions it wants.  If we return something unexpected, even it's
>> error_mark_node, then there's a higher risk of ICEs later on.
>> 
>> I guess that's a bit defeatist.  But in practice, the first code
>> that uses the hook will be code that previously ran at start-up
>> and so didn't have to worry about these errors.
>> 
>> In practice I think errors will be extremely rare.
>> 
 +  xref_basetypes (type, NULL_TREE);
 +  type = begin_class_definition (type);
 +  if (type == error_mark_node)
 +return lhd_simulate_record_decl (loc, name, fields);
 +
 +  for (tree field : fields)
 +finish_member_declaration (field);
 +
 +  type = finish_struct (type, NULL_TREE);
 +
 +  tree decl = build_decl (loc, TYPE_DECL, ident, type);
 +  TYPE_NAME (type) = decl;
 +  TYPE_STUB_DECL (type) = decl;
>>>
>>> Setting TYPE_NAME and TYPE_STUB_DECL to the typedef is wrong; it should
>>> work to just remove these two lines.  I expect they're also wrong for C.
>>>
>>> For C++ only, I wonder if you need this typedef at all.
>>>
>>> If you do want it, you need to use set_underlying_type to create a real
>>> typedef.  I expect that's also true for C.
>> 
>> Ah, yeah, thanks for the pointer.  Fixed in the patch below.
>> 
>> I wanted the hook to simulate the typedef even for C++ because its
>> first user will be arm_neon.h.  The spec for arm_neon.h says that the
>> types must be declared as:
>> 
>>typedef struct int32x2x4_t { … } int32x2x4_t;
>> 
>> etc.  So, although it's a silly edge case, code that tries to take
>> advantage of the struct stat hack, such as:
>> 
>>#include 
>>struct int32x2x4_t int32x2x4_t = {};
>> 
>> should continue to be rejected for C++ as well as C.
>> 
>> Maybe in future we could add a flag to suppress the typedef if some
>> callers prefer that behaviour.
>> 
>> Tested as before.
>
> Can the C++ hook go in cp-lang.c (which already includes 
> langhooks-def.h) instead of decl.c?  With that change, the patch is OK 
> in a week if nobody else has feedback.

Thanks.  I just tried with that change, but it breaks Objective C++ builds,
since cp-lang.o isn't linked there.

Richard

>
>> gcc/
>>  * langhooks.h (lang_hooks_for_types::simulate_record_decl): New hook.
>>  * langhooks-def.h (lhd_simulate_record_decl): Declare.
>>  (LANG_HOOKS_SIMULATE_RECORD_DECL): Define.
>>  (LANG_HOOKS_FOR_TYPES_INITIALIZER): Include it.
>>  * langhooks.c (lhd_simulate_record_decl): New function.
>> 
>> gcc/c/
>>  * c-tree.h (c_simulate_record_decl): Declare.
>>  * c-objc-common.h (LANG_HOOKS_SIMULATE_RECORD_DECL): Override.
>>  * c-decl.c (c_simulate_record_decl): New function.
>> 
>> gcc/cp/
>>  * decl.c: Include langhooks-def.h.
>>  (cxx_simulate_record_decl): New function.
>>  * cp-objcp-common.h (cxx_simulate_record_decl): Declare.
>>  (LANG_HOOKS_SIMULATE_RECORD_DECL): Override.
>> ---
>>   gcc/c/c-decl.c   | 30 ++
>>   gcc/c/c-objc-common.h|  2 ++
>>   gcc/c/c-tree.h   |  2 ++
>>   gcc/cp/cp-objcp-common.h |  4 
>>   gcc/cp/decl.c| 37 +
>>   gcc/langhooks-def.h  |  4 
>>   gcc/langhooks.c  | 19 +++
>>   gcc/langhooks.h  | 10 ++
>>   8 files changed, 108 insertions(+)
>> 
>> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
>> index 771efa3eadf..186fa1692c1 100644
>> --- a/gcc/c/c-decl.c
>> +++ b/gcc/c/c-decl.c
>> @@ -9436,6 +9436,36 @@ c_simulate_enum_decl (location_t loc, const char 
>> *name,
>> input_location = saved_loc;
>> return enumtype;
>>   }
>> +
>> +/* Implement LANG_HOOKS_SIMULATE_RECORD_DECL.  */
>> +
>> +tree
>> +c_simulate_record_decl (location_t loc, const char *name,
>> +array_slice fields)
>> +{
>> +  location_t saved_loc = input_location;
>> +  input_location = loc;
>> +
>> +  class c_struct_parse_info *struct_info;
>> +  tree ident = get_identifier (name);
>> +  tree type = start_struct (loc, RECORD_TYPE, ident, &struct_info);
>> +
>> +  for (unsigned int i = 0; i < fields.size (); ++i)
>> +{
>> +  DECL_FIELD_CONTEXT (fields[i]) = type;
>> +  if (i > 0)
>> +DECL_CHAIN (fields[i - 1]) = fields[i];
>> +}
>> +
>> +  finish_struct (loc, type, fields[0], NULL_TREE, struct_info);
>>

[committed] wwwdocs: gcc-4.7: Tweak AVR-Lib reference

2021-10-30 Thread Gerald Pfeifer 
nongnu.org has a permanent redirect (return code 301) to www.nongnu.org;
make that change on our end.
---
 htdocs/gcc-4.7/changes.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/htdocs/gcc-4.7/changes.html b/htdocs/gcc-4.7/changes.html
index dae1735d..21294cc3 100644
--- a/htdocs/gcc-4.7/changes.html
+++ b/htdocs/gcc-4.7/changes.html
@@ -740,7 +740,7 @@ int add_values (const __flash int *p, int i)
 }
 Support has been added for the AVR-specific configure option
   --with-avrlibc=yes in order to arrange for better
-  integration of http://nongnu.org/avr-libc/";>AVR-Libc.
+  integration of http://www.nongnu.org/avr-libc/";>AVR-Libc.
   This configure option is supported in avr-gcc 4.7.2 and newer and will
   only take effect in non-RTEMS configurations.  If avr-gcc is configured
   for RTEMS, the option will be ignored which is the same as
-- 
2.33.0

[PATCH] libsanitizer: Disable libbacktrace on sanitizer_platform_limits_freebsd.cpp

2021-10-30 Thread H.J. Lu via Gcc-patches 
sanitizer_platform_limits_freebsd.cpp must include  from the OS,
not include/md5.h in GCC source tree which is included by libbacktrace
support.  Disable libbacktrace on sanitizer_platform_limits_freebsd.cpp
to avoid include/md5.h to restore bootstrap on FreeBSD.

PR bootstrap/102675
* sanitizer_common/Makefile.am (AM_CXXFLAGS): Extract libbacktrace
CXXFLAGS to ...
(LIBBACKTRACE_CXXFLAGS): Here.  New.
(sanitizer_common_files): Move sanitizer_platform_limits_freebsd.cpp
to ...
(sanitizer_common_files_no_libbacktrace): Here.  New.
(AM_CXXFLAGS): Add $(LIBBACKTRACE_CXXFLAGS) for
$(sanitizer_common_files).
(libsanitizer_common_la_SOURCES): Add
$(sanitizer_common_files_no_libbacktrace).
* sanitizer_common/Makefile.in: Regenerate.
---
 libsanitizer/sanitizer_common/Makefile.am | 24 +++-
 libsanitizer/sanitizer_common/Makefile.in | 35 ---
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/libsanitizer/sanitizer_common/Makefile.am 
b/libsanitizer/sanitizer_common/Makefile.am
index d04f2d8bd16..0ea459c2b3a 100644
--- a/libsanitizer/sanitizer_common/Makefile.am
+++ b/libsanitizer/sanitizer_common/Makefile.am
@@ -9,11 +9,12 @@ AM_CXXFLAGS += $(LIBSTDCXX_RAW_CXX_CXXFLAGS)
 AM_CXXFLAGS += -std=gnu++14
 AM_CXXFLAGS += $(EXTRA_CXXFLAGS)
 if LIBBACKTRACE_SUPPORTED
-AM_CXXFLAGS += -DSANITIZER_LIBBACKTRACE -DSANITIZER_CP_DEMANGLE \
-  -I $(top_srcdir)/../libbacktrace \
-  -I $(top_builddir)/libbacktrace \
-  -I $(top_srcdir)/../include \
-  -include $(top_srcdir)/libbacktrace/backtrace-rename.h
+LIBBACKTRACE_CXXFLAGS = \
+  -DSANITIZER_LIBBACKTRACE -DSANITIZER_CP_DEMANGLE \
+  -I $(top_srcdir)/../libbacktrace \
+  -I $(top_builddir)/libbacktrace \
+  -I $(top_srcdir)/../include \
+  -include $(top_srcdir)/libbacktrace/backtrace-rename.h
 endif
 AM_CCASFLAGS = $(EXTRA_ASFLAGS)
 ACLOCAL_AMFLAGS = -I m4
@@ -45,7 +46,6 @@ sanitizer_common_files = \
sanitizer_netbsd.cpp \
sanitizer_openbsd.cpp \
sanitizer_persistent_allocator.cpp \
-   sanitizer_platform_limits_freebsd.cpp \
sanitizer_platform_limits_linux.cpp \
sanitizer_platform_limits_openbsd.cpp \
sanitizer_platform_limits_posix.cpp \
@@ -81,8 +81,18 @@ sanitizer_common_files = \
sanitizer_unwind_win.cpp \
sanitizer_win.cpp
 
+# Don't add $(LIBBACKTRACE_CXXFLAGS) for the following files:
+# 1. sanitizer_platform_limits_freebsd.cpp must include  from
+#the OS, not include/md5.h in GCC source tree.
+sanitizer_common_files_no_libbacktrace = \
+   sanitizer_platform_limits_freebsd.cpp
 
-libsanitizer_common_la_SOURCES = $(sanitizer_common_files) 
+$(sanitizer_common_files:.cpp=.lo) \
+  $(sanitizer_common_files:.cpp=.$(OBJEXT)): AM_CXXFLAGS += 
$(LIBBACKTRACE_CXXFLAGS)
+
+libsanitizer_common_la_SOURCES = \
+  $(sanitizer_common_files) \
+  $(sanitizer_common_files_no_libbacktrace)
 libsanitizer_common_la_LIBADD = $(SANITIZER_COMMON_TARGET_DEPENDENT_OBJECTS)
 libsanitizer_common_la_DEPENDENCIES =  
$(SANITIZER_COMMON_TARGET_DEPENDENT_OBJECTS)
 
diff --git a/libsanitizer/sanitizer_common/Makefile.in 
b/libsanitizer/sanitizer_common/Makefile.in
index 2856894d62b..1433db2238b 100644
--- a/libsanitizer/sanitizer_common/Makefile.in
+++ b/libsanitizer/sanitizer_common/Makefile.in
@@ -89,12 +89,6 @@ POST_UNINSTALL = :
 build_triplet = @build@
 host_triplet = @host@
 target_triplet = @target@
-@LIBBACKTRACE_SUPPORTED_TRUE@am__append_1 = -DSANITIZER_LIBBACKTRACE 
-DSANITIZER_CP_DEMANGLE \
-@LIBBACKTRACE_SUPPORTED_TRUE@ -I $(top_srcdir)/../libbacktrace \
-@LIBBACKTRACE_SUPPORTED_TRUE@ -I $(top_builddir)/libbacktrace \
-@LIBBACKTRACE_SUPPORTED_TRUE@ -I $(top_srcdir)/../include \
-@LIBBACKTRACE_SUPPORTED_TRUE@ -include 
$(top_srcdir)/libbacktrace/backtrace-rename.h
-
 subdir = sanitizer_common
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../config/acx.m4 \
@@ -131,7 +125,6 @@ am__objects_1 = sancov_flags.lo sanitizer_allocator.lo \
sanitizer_mac.lo sanitizer_mac_libcdep.lo sanitizer_mutex.lo \
sanitizer_netbsd.lo sanitizer_openbsd.lo \
sanitizer_persistent_allocator.lo \
-   sanitizer_platform_limits_freebsd.lo \
sanitizer_platform_limits_linux.lo \
sanitizer_platform_limits_openbsd.lo \
sanitizer_platform_limits_posix.lo \
@@ -153,7 +146,8 @@ am__objects_1 = sancov_flags.lo sanitizer_allocator.lo \
sanitizer_thread_registry.lo sanitizer_tls_get_addr.lo \
sanitizer_unwind_linux_libcdep.lo sanitizer_unwind_win.lo \
sanitizer_win.lo
-am_libsanitizer_common_la_OBJECTS = $(am__objects_1)
+am__objects_2 = sanitizer_platform_limits_freebsd.lo
+am_libsanitizer_common_la_OBJECTS = $(am__objects_1) $(am__objects_2)
 libsanitizer_common_la_OBJECTS = $(am_libsanitizer_common_la_OBJECTS)
 AM_V_lt = $(am__

Re: [Patch] OpenMP: Add strictly nested API call check [PR102972]

2021-10-30 Thread Tobias Burnus 

On 29.10.21 18:47, Jakub Jelinek wrote:

Or we can keep 3 sections and say that the first one is for the
calls on the library side without suffixes and second is for those with
no and _ suffixes, but that in DECL_NAME those don't make a difference.

That's what I have now done.

+  && strncmp (IDENTIFIER_POINTER (DECL_NAME (fndecl)),
+  "omp_get_num_teams",
+  strlen ("omp_get_num_teams")) != 0
+  && strncmp (IDENTIFIER_POINTER (DECL_NAME (fndecl)),
+  "omp_get_team_num",
+  strlen ("omp_get_team_num")) != 0)

If we wanted to optimize, we could decide based on IDENTIFIER_LENGTH whether
to use strncmp at all and which one.

I did this optimization and moved to strcmp has there is no _ suffix in
general and no _8 suffix in particular.

Can't we use instead
   #pragma omp distribute dist_schedule(static,1)
   for (int i = 0; i < omp_get_num_teams (); ++i)
which I believe should ensure that each team will execute exactly one
iteration (i.e. exactly what the code has been doing before).

Did use this now.

Otherwise LGTM.


Thanks for the review. Committed as Rev.
r12-4809-g948d461954f2642ca187f86c19d297ba7a86320f

Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
commit 948d461954f2642ca187f86c19d297ba7a86320f
Author: Tobias Burnus 
Date:   Sat Oct 30 23:45:32 2021 +0200

OpenMP: Add strictly nested API call check [PR102972]

The teams construct only permits omp_get_num_teams and omp_get_team_num
as API call in strictly nested regions - check for it.

Additionally, for Fortran, using DECL_NAME does not show the mangled
name, hence, DECL_ASSEMBLER_NAME had to be used to.

Finally, 'target device(ancestor:1)' wrongly rejected non-API calls
as well.

PR middle-end/102972
gcc/ChangeLog:

* omp-low.c (omp_runtime_api_call): Use DECL_ASSEMBLER_NAME to get
internal Fortran name; new permit_num_teams arg to permit
omp_get_num_teams and omp_get_team_num.
(scan_omp_1_stmt): Update call to it, add missing call for
reverse offload, and check for strictly nested API calls in teams.

gcc/testsuite/ChangeLog:

* c-c++-common/gomp/target-device-ancestor-3.c: Add non-API
routine test.
* gfortran.dg/gomp/order-6.f90: Add missing bind(C).
* c-c++-common/gomp/teams-3.c: New test.
* gfortran.dg/gomp/teams-3.f90: New test.
* gfortran.dg/gomp/teams-4.f90: New test.

libgomp/ChangeLog:
* testsuite/libgomp.c-c++-common/icv-3.c: Nest API calls inside
parallel construct.
* testsuite/libgomp.c-c++-common/icv-4.c: Likewise.
* testsuite/libgomp.c/target-3.c: Likewise.
* testsuite/libgomp.c/target-5.c: Likewise.
* testsuite/libgomp.c/target-6.c: Likewise.
* testsuite/libgomp.c/target-teams-1.c: Likewise.
* testsuite/libgomp.c/teams-1.c: Likewise.
* testsuite/libgomp.c/thread-limit-2.c: Likewise.
* testsuite/libgomp.c/thread-limit-3.c: Likewise.
* testsuite/libgomp.c/thread-limit-4.c: Likewise.
* testsuite/libgomp.c/thread-limit-5.c: Likewise.
* testsuite/libgomp.fortran/icv-3.f90: Likewise.
* testsuite/libgomp.fortran/icv-4.f90: Likewise.
* testsuite/libgomp.fortran/teams1.f90: Likewise.
---
 gcc/omp-low.c  |  31 --
 .../c-c++-common/gomp/target-device-ancestor-3.c   |   2 +
 gcc/testsuite/c-c++-common/gomp/teams-3.c  |  64 
 gcc/testsuite/gfortran.dg/gomp/order-6.f90 |   2 +-
 gcc/testsuite/gfortran.dg/gomp/teams-3.f90 |  65 
 gcc/testsuite/gfortran.dg/gomp/teams-4.f90 |  47 +
 libgomp/testsuite/libgomp.c-c++-common/icv-3.c |   3 +
 libgomp/testsuite/libgomp.c-c++-common/icv-4.c |   1 +
 libgomp/testsuite/libgomp.c/target-3.c |   6 +-
 libgomp/testsuite/libgomp.c/target-5.c |   1 +
 libgomp/testsuite/libgomp.c/target-6.c |  12 ++-
 libgomp/testsuite/libgomp.c/target-teams-1.c   | 115 +++--
 libgomp/testsuite/libgomp.c/teams-1.c  |   6 +-
 libgomp/testsuite/libgomp.c/thread-limit-2.c   |  21 ++--
 libgomp/testsuite/libgomp.c/thread-limit-3.c   |   1 +
 libgomp/testsuite/libgomp.c/thread-limit-4.c   |  25 +++--
 libgomp/testsuite/libgomp.c/thread-limit-5.c   |   1 +
 libgomp/testsuite/libgomp.fortran/icv-3.f90|   6 ++
 libgomp/testsuite/libgomp.fortran/icv-4.f90|   2 +
 libgomp/testsuite/libgomp.fortran/teams1.

Re: [PATCH 06/18] rs6000: Builtin expansion, part 1

2021-10-30 Thread Segher Boessenkool 
Hi!

On Wed, Sep 01, 2021 at 11:13:42AM -0500, Bill Schmidt wrote:
> Differences between the old and new support in this patch include:
>  - Make use of the new builtin data structures, directly looking up
>a function's information rather than searching for the function
>multiple times;

Is that measurable, do you think?

>  - Test for enablement of builtins at expand time, to support #pragma
>target changes within a compilation unit;

But not within a function, right?

> Note that these six patches must be pushed together, because otherwise
> unused parameter warnings in the stub functions will prevent bootstrap.

Must be *committed* as one commit, even.  Committing them as six
separate commits and pushing them all at once will do nothing for people
who try to bisect, etc.

Merging patches is easy with Git though :-)

> +/* Expand ALTIVEC_BUILTIN_MASK_FOR_LOAD.  */
> +rtx
> +rs6000_expand_ldst_mask (rtx target, tree arg0)
> + {
> +  return target;
> + }

Interesting leading spaces, heh.  Please fix.

> +/* Expand the HTM builtin in EXP and store the result in TARGET.  */
> +static rtx
> +new_htm_expand_builtin (bifdata *bifaddr, rs6000_gen_builtins fcode,
> + tree exp, rtx target)
> +{
> +  return const0_rtx;
> +}

The function commment should say what the return value is.

> +/* Expand an expression EXP that calls a built-in function,
> +   with result going to TARGET if that's convenient
> +   (and in mode MODE if that's convenient).
> +   SUBTARGET may be used as the target for computing one of EXP's operands.
> +   IGNORE is nonzero if the value is to be ignored.
> +   Use the new builtin infrastructure.  */
> +static rtx
> +rs6000_expand_new_builtin (tree exp, rtx target,
> +rtx subtarget ATTRIBUTE_UNUSED,
> +machine_mode ignore_mode ATTRIBUTE_UNUSED,
> +int ignore ATTRIBUTE_UNUSED)

Don't use ATTRIBUTE_UNUSED?  We have C++ now, you can leave out the
parameter name, with the same effect (other than it does not make you go
blind ;-) ).  In the case where you still want to show the name, you can
do something like
rs6000_expand_new_builtin (tree exp, trx target, rtx /*subtarget*/,
machine_mode, int /*ignore*/)

(There is no argument MODE btw, the comment needs some tweaking).

> +  /* We have two different modes (KFmode, TFmode) that are the IEEE
> + 128-bit floating point type, depending on whether long double is the
> + IBM extended double (KFmode) or long double is IEEE 128-bit (TFmode).

KFmode *always* is IEEE QP.  TFmode is the one that can be different.

> + It is simpler if we only define one variant of the built-in function,
> + and switch the code when defining it, rather than defining two built-
> + ins and using the overload table in rs6000-c.c to switch between the
> + two.  If we don't have the proper assembler, don't do this switch
> + because CODE_FOR_*kf* and CODE_FOR_*tf* will be CODE_FOR_nothing.  */
> +  if (FLOAT128_IEEE_P (TFmode))
> +switch (icode)
> +  {
> +  default:
> + break;

default: goes at the *end*.  And you can usually leave it out.

> +  case CODE_FOR_sqrtkf2_odd:
> + icode = CODE_FOR_sqrttf2_odd;
> + break;

So please do this the other way?  In libgcc "tf" means double-double,
it is historical.  So let's do the clearer thing please: translate tf to
kf in this handling (when tf *does* mean kf ;-) )

> +  /* In case of "#pragma target" changes, we initialize all builtins
> + but check for actual availability now, during expand time.  For
> + invalid builtins, generate a normal call.  */
> +  bifdata *bifaddr = &rs6000_builtin_info_x[uns_fcode];
> +  bif_enable e = bifaddr->enable;
> +
> +  if (e != ENB_ALWAYS
> +  && (e != ENB_P5   || !TARGET_POPCNTB)

  if (!(e == ENB_ALWAYS
|| (e == ENB_P5 && TARGET_POPCNTB)

etc.

Computers are better at De Morgan than humans are.  It is much more
important to write clear code.  This often means using fewer negations.

> +  const int MAX_BUILTIN_ARGS = 6;
> +  tree arg[MAX_BUILTIN_ARGS];
> +  rtx op[MAX_BUILTIN_ARGS];
> +  machine_mode mode[MAX_BUILTIN_ARGS + 1];

Arrays are always better with a short comment.
Why is the "mode" array one entry longer, btw?

> +  rtx pat;
> +  bool void_func = TREE_TYPE (TREE_TYPE (fndecl)) == void_type_node;
> +  int k;

These things should be declared where they are first used.  The
initialisation of void_func should not be hidden amidst boring
declarations.

"k" needs a comment.

> +  if (!insn_data[icode].operand[i+k].mode)
> + mode[i+k] = TARGET_64BIT ? Pmode : SImode;

That is
  mode[i+k] = Pmode;
always.

Does this depend on VOIDmode being equal to 0?  That is guaranteed, but
if you write out VOIDmode elsewhere, do it here as well?

> +  else
> + mode[i+k] = insn_data[icode].operand[i+k].mode;
> +}

So this is
  mode[i+k] = insn_data[icode].operand[i+k].mode;
  if (!m