Bug 108384 - [13 Regression] error: conversion of register to a different size in ‘view_convert_expr’
Summary: [13 Regression] error: conversion of register to a different size in ‘view_co...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ipa (show other bugs)
Version: 13.0
: P1 normal
Target Milestone: 13.0
Assignee: Martin Jambor
URL:
Keywords: ice-checking, ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2023-01-12 14:53 UTC by David Binderman
Modified: 2023-02-27 23:07 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-01-12 00:00:00


Attachments
C source code (43.30 KB, text/plain)
2023-01-12 14:53 UTC, David Binderman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Binderman 2023-01-12 14:53:16 UTC
Created attachment 54256 [details]
C source code

The attached C code does this:

$ ~/gcc/results/bin/gcc -c -w -O3 -ftrivial-auto-var-init=zero bug873.c
testFile.4557.c: In function ‘func_1.isra’:
testFile.4557.c:130:16: error: conversion of register to a different size in ‘view_convert_expr’
VIEW_CONVERT_EXPR<short int>(removed_ipa_cp.834_390);

_393 = VIEW_CONVERT_EXPR<short int>(removed_ipa_cp.834_390);
during GIMPLE pass: fixup_cfg
testFile.4557.c:130:16: internal compiler error: verify_gimple failed
0xeaab49 verify_gimple_in_cfg(function*, bool, bool)
	../../trunk.d1/gcc/tree-cfg.cc:5647
0xd46c5b execute_function_todo(function*, void*)
	../../trunk.d1/gcc/passes.cc:2091

The bug first seems to occur sometime between git hash g:d901bf8a44a85e12
and g:b399afd22c6ea507.

A reduction is running now.
Comment 1 David Binderman 2023-01-12 14:56:20 UTC
Reduced C code seems to be:

struct S0 {
  int f0;
  short f1;
  unsigned f2 : 7;
  short f3
};
g_389;
static *func_23();
func_2() {
  struct S0 l_26[] = {4, 5, 4, 6, 4, 5, 4, 6};
  func_23(l_26[1]);
}
*func_23(struct S0 p_24, struct S0 p_25) {
  int *l_1051 = g_389;
  if (safe_sub_func_int16_t_s_s())
    safe_lshift_func_uint8_t_u_s(p_24.f1);
  *l_1051 = p_25.f0;
}
Comment 2 Andrew Pinski 2023-01-12 16:11:54 UTC
The code is undefined ...

  func_23(l_26[1]);

func_23(struct S0 p_24, struct S0 p_25)
Comment 3 David Binderman 2023-01-12 16:26:28 UTC
(In reply to Andrew Pinski from comment #2)
> The code is undefined ...
> 
>   func_23(l_26[1]);
> 
> func_23(struct S0 p_24, struct S0 p_25)

Interesting. It looks like the reduction has not preserved the two
parameters required of func_23.

From the original csmith produced code:

$ fgrep func_23 bug873.c
static union U2 * func_23(struct S0 p_24, const struct S1 p_25);
    if ((((*l_9) &= 0x3631L) , (safe_div_func_int32_t_s_s((safe_sub_func_int8_t_s_s((((safe_mod_func_uint32_t_u_u(((func_17(func_23(l_26[1], l_27), l_9, ((*l_1367) = l_1366), (safe_add_func_uint32_t_u_u(((safe_div_func_uint8_t_u_u(((safe_rshift_func_uint8_t_u_s((l_27.f0 > p_3), 1)) || (((safe_div_func_int64_t_s_s(p_3, p_3)) != 0x47247D4EED584808LL) || g_1119)), 0x21L)) > g_1165[0]), 0xB3A4A36DL)), l_27.f4.f0) <= 0x334C102EE31FC4EFLL) & p_3), g_7.f0)) & g_1165[0]) || l_1397), 0L)), 0x55B9621EL))))
static union U2 * func_23(struct S0 p_24, const struct S1 p_25)

Two parameters in the first declaration, two in the call and two in the definition.
Comment 4 David Binderman 2023-01-12 17:05:18 UTC
(In reply to David Binderman from comment #3)
> (In reply to Andrew Pinski from comment #2)
> > The code is undefined ...
> > 
> >   func_23(l_26[1]);
> > 
> > func_23(struct S0 p_24, struct S0 p_25)
> 
> Interesting. It looks like the reduction has not preserved the two
> parameters required of func_23.

I suspect a grep pattern could help guide the reduction.
I tried a few patterns, but didn't make any real progress.

Meanwhile, I try a bisection. Trying git hash g:0333892db367b2b9
Comment 5 David Binderman 2023-01-12 17:14:41 UTC
(In reply to David Binderman from comment #4)
> Meanwhile, I try a bisection. Trying git hash g:0333892db367b2b9

Seems good. Trying g:d3328df5f5c9908c
Comment 6 David Binderman 2023-01-12 17:19:39 UTC
(In reply to David Binderman from comment #5)
> (In reply to David Binderman from comment #4)
> > Meanwhile, I try a bisection. Trying git hash g:0333892db367b2b9
> 
> Seems good. Trying g:d3328df5f5c9908c

Seems good. Trying g:5b918b20d18b9cce
Comment 7 David Binderman 2023-01-12 17:35:56 UTC
(In reply to David Binderman from comment #6)
> (In reply to David Binderman from comment #5)
> > (In reply to David Binderman from comment #4)
> > > Meanwhile, I try a bisection. Trying git hash g:0333892db367b2b9
> > 
> > Seems good. Trying g:d3328df5f5c9908c
> 
> Seems good. Trying g:5b918b20d18b9cce

It appears to me that Martin Jambor's revision g:c389991432da2bcc
is the one at fault.

Adding Martin for their best opinion.
Comment 8 David Binderman 2023-01-12 18:32:25 UTC
(In reply to David Binderman from comment #4)
> I suspect a grep pattern could help guide the reduction.
> I tried a few patterns, but didn't make any real progress.

Using this pattern:

$ grep "func_23[^,]*,[^)]*)" bug873.c

I got the following reduced code:

struct S0 {
  int f0;
  short f1;
  unsigned f2 : 7;
  short f3
} func_2_l_27;
g_389;
func_23(struct S0 p_24, struct S0 p_25) {
  int *l_1051 = g_389;
  if (safe_sub_func_int16_t_s_s())
    for (;;)
      safe_lshift_func_uint8_t_u_s(p_24.f1);
  *l_1051 = p_25.f0;
}
func_2() {
  struct S0 l_26[] = {4, 5, 4, 6, 4, 5, 4, 6};
  func_23(l_26[1], func_2_l_27);
}

$ fgrep func_23 bug873.c
func_23(struct S0 p_24, struct S0 p_25) {
  func_23(l_26[1], func_2_l_27);
$
Comment 9 Andrew Pinski 2023-01-12 19:29:30 UTC
Confirmed.
Here is a cleaned up testcase where you don't need the -ftrivial-auto-var-init=zero option either; just -O3

```
struct S0 {
  int f0;
  short f1;
  unsigned f2 : 7;
  short f3;
} func_2_l_27;
int *g_389;
int safe_sub_func_int16_t_s_s(void);
void safe_lshift_func_uint8_t_u_s(int);
void func_23(struct S0 p_24, struct S0 p_25) {
  int *l_1051 = g_389;
  if (safe_sub_func_int16_t_s_s())
    for (;;)
      safe_lshift_func_uint8_t_u_s(p_24.f1);
  *l_1051 = p_25.f0;
}
void func_2(void) {
  struct S0 l_26[] = {4, 5, 4, 6, 4, 5, 4, 6};
  __builtin_clear_padding (&l_26);
  func_23(l_26[1], func_2_l_27);
}
```
Comment 10 Jakub Jelinek 2023-01-12 19:30:42 UTC
If it is __builtin_clear_padding only, I'll have a look...
Comment 11 Andrew Pinski 2023-01-12 19:43:29 UTC
(In reply to Jakub Jelinek from comment #10)
> If it is __builtin_clear_padding only, I'll have a look...

After the __builtin_clear_padding is exposed and before the IPA passes, the IR looks like:

  l_26[1].f0 = 4;
  MEM <long int> [(struct S0[2] *)&l_26 + 16B] = 25770065925;
  func_23 (l_26[1], func_2_l_27);

So here is another testcase which you can reproduce it with -O3 -fno-strict-aliasing (because I voilate aliasing rules to get a similar enough IR):
```
struct S0 {
  int f0;
  short f1;
  unsigned f2 : 7;
  short f3;
} func_2_l_27;
int *g_389;
int safe_sub_func_int16_t_s_s(void);
void safe_lshift_func_uint8_t_u_s(int);
void func_23(struct S0 p_24, struct S0 p_25) {
  int *l_1051 = g_389;
  if (safe_sub_func_int16_t_s_s())
    for (;;)
      safe_lshift_func_uint8_t_u_s(p_24.f1);
  *l_1051 = p_25.f0;
}
void func_2(void) {
  struct S0 l_26[2];
  l_26[1].f0 = 4;
  ((long long*)&l_26)[2] = 25770065925;
  func_23(l_26[1], func_2_l_27);
}
```
Comment 12 Martin Jambor 2023-01-12 21:11:22 UTC
Looks like V_C_E insertion by ipa_param_body_adjustments needs to be more careful.  Let me have a look.
Comment 13 Martin Jambor 2023-02-02 16:20:43 UTC
I have proposed a fix on the mailing list:

https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611194.html
Comment 14 GCC Commits 2023-02-03 12:30:23 UTC
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:e8109bd87766be88e83fe88a44433dae16358a02

commit r13-5681-ge8109bd87766be88e83fe88a44433dae16358a02
Author: Martin Jambor <mjambor@suse.cz>
Date:   Fri Feb 3 13:28:24 2023 +0100

    ipa: Avoid invalid gimple when IPA-CP and IPA-SRA disagree on types (108384)
    
    When the compiled program contains type mismatches between callers and
    callees when it comes to a parameter, IPA-CP can try to propagate one
    constant from callers while IPA-SRA may try to split a parameter
    expecting a value of a different size on the same offset.  This then
    currently leads to creation of a VIEW_CONVERT_EXPR with mismatching
    type sizes of LHS and RHS which is correctly flagged by the GIMPLE
    verifier as invalid.
    
    It seems that the best course of action is to try and avoid the
    situation altogether and so this patch adds a check to IPA-SRA that
    peeks into the result of IPA-CP and when it sees a value on the same
    offset but with a mismatching size, it just decides to leave that
    particular parameter be.
    
    gcc/ChangeLog:
    
    2023-02-02  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/108384
            * ipa-sra.cc (push_param_adjustments_for_index): Remove a size check
            when comparing to an IPA-CP value.
            (dump_list_of_param_indices): New function.
            (adjust_parameter_descriptions): Check for mismatching IPA-CP values.
            Dump removed candidates using dump_list_of_param_indices.
            * ipa-param-manipulation.cc
            (ipa_param_body_adjustments::modify_expression): Add assert checking
            sizes of a VIEW_CONVERT_EXPR will match.
            (ipa_param_body_adjustments::modify_assignment): Likewise.
    
    gcc/testsuite/ChangeLog:
    
    2023-02-02  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/108384
            * gcc.dg/ipa/pr108384.c: New test.
Comment 15 Martin Jambor 2023-02-03 12:33:03 UTC
Fixed, thanks for reporting.